diff --git a/bin/system_info/print_system_info.c b/bin/system_info/print_system_info.c index 5c86b8187..c29877086 100644 --- a/bin/system_info/print_system_info.c +++ b/bin/system_info/print_system_info.c @@ -1,9 +1,8 @@ - -#include #include #include +#include int main(void) { struct aws_allocator *allocator = aws_default_allocator(); @@ -22,10 +21,15 @@ int main(void) { fprintf(stdout, "crt-detected env: {\n"); struct aws_byte_cursor virtualization_vendor = aws_system_environment_get_virtualization_vendor(env); - fprintf(stdout, " 'virtualization vendor': '" PRInSTR "',\n", (int)virtualization_vendor.len, virtualization_vendor.ptr); + fprintf( + stdout, + " 'virtualization vendor': '" PRInSTR "',\n", + (int)virtualization_vendor.len, + virtualization_vendor.ptr); struct aws_byte_cursor product_name = aws_system_environment_get_virtualization_product_name(env); fprintf(stdout, " 'product name': '" PRInSTR "',\n", (int)product_name.len, product_name.ptr); - fprintf(stdout, " 'number of processors': '%lu',\n", (unsigned long)aws_system_environment_get_processor_count(env)); + fprintf( + stdout, " 'number of processors': '%lu',\n", (unsigned long)aws_system_environment_get_processor_count(env)); size_t numa_nodes = aws_system_environment_get_cpu_group_count(env); if (numa_nodes > 1) { @@ -36,7 +40,7 @@ int main(void) { } fprintf(stdout, "}\n"); - aws_system_environment_destroy(env); + aws_system_environment_release(env); aws_logger_clean_up(&logger); aws_common_library_clean_up(); diff --git a/include/aws/common/private/system_info_priv.h b/include/aws/common/private/system_info_priv.h index 608bd056c..27b1d4ad1 100644 --- a/include/aws/common/private/system_info_priv.h +++ b/include/aws/common/private/system_info_priv.h @@ -4,13 +4,14 @@ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. * SPDX-License-Identifier: Apache-2.0. */ - #include +#include #include #include struct aws_system_environment { struct aws_allocator *allocator; + struct aws_ref_count ref_count; struct aws_byte_buf virtualization_vendor; struct aws_byte_buf product_name; enum aws_platform_os os; diff --git a/include/aws/common/system_info.h b/include/aws/common/system_info.h index 5e840ad35..be4554fd4 100644 --- a/include/aws/common/system_info.h +++ b/include/aws/common/system_info.h @@ -34,7 +34,10 @@ AWS_COMMON_API struct aws_system_environment *aws_system_environment_load(struct aws_allocator *allocator); AWS_COMMON_API -void aws_system_environment_destroy(struct aws_system_environment *env); +struct aws_system_environment *aws_system_environment_acquire(struct aws_system_environment *env); + +AWS_COMMON_API +void aws_system_environment_release(struct aws_system_environment *env); /** * Returns the virtualization vendor for the specified compute environment, e.g. "Xen, Amazon EC2, etc..." @@ -62,7 +65,7 @@ size_t aws_system_environment_get_processor_count(struct aws_system_environment * Returns the number of separate cpu groupings (multi-socket configurations or NUMA). */ AWS_COMMON_API -size_t aws_system_environment_get_cpu_group_count(struct aws_system_environment *env); +size_t aws_system_environment_get_cpu_group_count(const struct aws_system_environment *env); /* Returns the OS this was built under */ AWS_COMMON_API diff --git a/source/file.c b/source/file.c index 264bbfe5f..e29458146 100644 --- a/source/file.c +++ b/source/file.c @@ -62,39 +62,43 @@ int aws_byte_buf_init_from_file(struct aws_byte_buf *out_buf, struct aws_allocat goto error; } + /* + * This number is usually correct, but in cases of device files that don't correspond to storage on disk, + * it may just be the size of a page. Go ahead and use it as a good hint of how much to allocate initially, + * but otherwise don't rely on it. + */ size_t allocation_size = (size_t)len64 + 1; aws_byte_buf_init(out_buf, alloc, allocation_size); - /* Ensure compatibility with null-terminated APIs, but don't consider - * the null terminator part of the length of the payload */ - out_buf->len = out_buf->capacity - 1; - out_buf->buffer[out_buf->len] = 0; - - size_t read = fread(out_buf->buffer, 1, out_buf->len, fp); - if (read < out_buf->len) { - int errno_value = ferror(fp) ? errno : 0; /* Always cache errno before potential side-effect */ - if (errno_value != 0) { - aws_translate_and_raise_io_error_or(errno_value, AWS_ERROR_FILE_READ_FAILURE); - AWS_LOGF_ERROR( - AWS_LS_COMMON_IO, - "static: Failed reading file:'%s' errno:%d aws-error:%s", - filename, - errno_value, - aws_error_name(aws_last_error())); - goto error; - } else { - AWS_LOGF_WARN( - AWS_LS_COMMON_IO, - "static: reading file:'%s' reports longer length %d than actually reading from it: %d. This is not " - "necessarily an error as devices will usually report a page for length regardless of actual length.", - filename, - (int)read, - (int)out_buf->len); - out_buf->len = read; + size_t read = -1; + size_t total_read = 0; + do { + if (total_read == out_buf->capacity) { + /* just add allocation size space to read some more. It's not perfect but it's plenty good. */ + aws_byte_buf_reserve_relative(out_buf, allocation_size); } + read = fread(out_buf->buffer + out_buf->len, 1, out_buf->capacity - out_buf->len, fp); + out_buf->len += read; + total_read += read; + } while (read > 0); + + int errno_value = ferror(fp) ? errno : 0; /* Always cache errno before potential side-effect */ + if (errno_value != 0) { + aws_translate_and_raise_io_error_or(errno_value, AWS_ERROR_FILE_READ_FAILURE); + AWS_LOGF_ERROR( + AWS_LS_COMMON_IO, + "static: Failed reading file:'%s' errno:%d aws-error:%s", + filename, + errno_value, + aws_error_name(aws_last_error())); + goto error; } fclose(fp); + /* write the NULL terminator out. */ + aws_byte_buf_write_u8(out_buf, 0x00); + /* we wrote the NULL terminator, but don't include it in the length. */ + out_buf->len -= 1; return AWS_OP_SUCCESS; error: diff --git a/source/system_info.c b/source/system_info.c index dcc4b57a8..4b721f63a 100644 --- a/source/system_info.c +++ b/source/system_info.c @@ -6,9 +6,19 @@ #include +void s_destroy_env(void *arg) { + struct aws_system_environment *env = arg; + + if (env) { + aws_system_environment_destroy_platform_impl(env); + aws_mem_release(env->allocator, env); + } +} + struct aws_system_environment *aws_system_environment_load(struct aws_allocator *allocator) { struct aws_system_environment *env = aws_mem_calloc(allocator, 1, sizeof(struct aws_system_environment)); env->allocator = allocator; + aws_ref_count_init(&env->ref_count, env, s_destroy_env); if (aws_system_environment_load_platform_impl(env)) { AWS_LOGF_ERROR( @@ -36,16 +46,17 @@ struct aws_system_environment *aws_system_environment_load(struct aws_allocator return env; error: - aws_mem_release(allocator, env); - + s_destroy_env(env); return NULL; } -void aws_system_environment_destroy(struct aws_system_environment *env) { - if (env) { - aws_system_environment_destroy_platform_impl(env); - aws_mem_release(env->allocator, env); - } +struct aws_system_environment *aws_system_environment_acquire(struct aws_system_environment *env) { + aws_ref_count_acquire(&env->ref_count); + return env; +} + +void aws_system_environment_release(struct aws_system_environment *env) { + aws_ref_count_release(&env->ref_count); } struct aws_byte_cursor aws_system_environment_get_virtualization_vendor(const struct aws_system_environment *env) { @@ -64,6 +75,6 @@ size_t aws_system_environment_get_processor_count(struct aws_system_environment } AWS_COMMON_API -size_t aws_system_environment_get_cpu_group_count(struct aws_system_environment *env) { +size_t aws_system_environment_get_cpu_group_count(const struct aws_system_environment *env) { return env->cpu_group_count; } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 9a186f750..8d254bfc2 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -481,6 +481,7 @@ add_test_case(directory_move_src_non_existent_test) add_test_case(test_home_directory_not_null) add_test_case(test_normalize_posix_directory_separator) add_test_case(test_normalize_windows_directory_separator) +add_test_case(test_byte_buf_file_read) add_test_case(promise_test_wait_forever) add_test_case(promise_test_wait_for_a_bit) diff --git a/tests/file_test.c b/tests/file_test.c index 6eedd264e..6205a6f89 100644 --- a/tests/file_test.c +++ b/tests/file_test.c @@ -439,3 +439,31 @@ static int s_test_normalize_windows_directory_separator(struct aws_allocator *al } AWS_TEST_CASE(test_normalize_windows_directory_separator, s_test_normalize_windows_directory_separator); + +static int s_test_byte_buf_file_read(struct aws_allocator *allocator, void *ctx) { + (void)ctx; + + const char *test_string = "This is a message that's going to test a read loop."; + struct aws_byte_buf test_buf = aws_byte_buf_from_c_str(test_string); + struct aws_string *test_file = aws_string_new_from_c_str(allocator, "test_file.txt"); + struct aws_string *test_file_mode = aws_string_new_from_c_str(allocator, "w"); + + FILE *output_file = aws_fopen("test_file", "w"); + ASSERT_NOT_NULL(output_file); + ASSERT_UINT_EQUALS(test_buf.len, fwrite(test_buf.buffer, 1, test_buf.len, output_file)); + fclose(output_file); + + struct aws_byte_buf output_buf; + AWS_ZERO_STRUCT(output_buf); + ASSERT_SUCCESS(aws_byte_buf_init_from_file(&output_buf, allocator, "test_file")); + aws_file_delete(test_file); + ASSERT_BIN_ARRAYS_EQUALS(test_buf.buffer, test_buf.len, output_buf.buffer, output_buf.len); + + aws_byte_buf_clean_up(&output_buf); + aws_string_destroy(test_file_mode); + aws_string_destroy(test_file); + + return AWS_OP_SUCCESS; +} + +AWS_TEST_CASE(test_byte_buf_file_read, s_test_byte_buf_file_read); diff --git a/tests/system_info_tests.c b/tests/system_info_tests.c index 571b6cbc5..7a4324797 100644 --- a/tests/system_info_tests.c +++ b/tests/system_info_tests.c @@ -178,7 +178,7 @@ static int s_test_sanity_check_environment_loader(struct aws_allocator *allocato struct aws_byte_cursor virt_product = aws_system_environment_get_virtualization_product_name(env); ASSERT_TRUE(aws_byte_cursor_is_valid(&virt_product)); - aws_system_environment_destroy(env); + aws_system_environment_release(env); aws_common_library_clean_up(); return 0;