Skip to content

Commit c31e7ed

Browse files
SamuelMarksSamuelMarks
committed
[acquire/acquire_*.h] Minor improvements to error outputting (stop stderr just add it to handle) ; [acquire/tests/CMakeLists.txt] Remove potential One-Definition Rule violations (and simplify!)
1 parent 833e379 commit c31e7ed

File tree

10 files changed

+63
-63
lines changed

10 files changed

+63
-63
lines changed

‎acquire/acquire_crc32c.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,6 @@ int _crc32c_verify_async_start(struct acquire_handle *handle,
122122
{
123123
const errno_t err = fopen_s(&be->file, filepath, "rb");
124124
if (err != 0 || be->file == NULL) {
125-
fprintf(stderr, "couldn't open file for reading %s\n", filepath);
126125
acquire_handle_set_error(handle, ACQUIRE_ERROR_FILE_OPEN_FAILED,
127126
"Cannot open file: %s", filepath);
128127
free(be);

‎acquire/acquire_libarchive.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ int acquire_extract_async_start(struct acquire_handle *handle,
9494
}
9595
handle->backend_handle = be;
9696
strncpy(be->dest_path, dest_path, sizeof(be->dest_path) - 1);
97+
be->dest_path[sizeof(be->dest_path) - 1] = '\0';
9798

9899
flags = ARCHIVE_EXTRACT_TIME | ARCHIVE_EXTRACT_PERM | ARCHIVE_EXTRACT_ACL |
99100
ARCHIVE_EXTRACT_FFLAGS;
@@ -144,6 +145,7 @@ enum acquire_status acquire_extract_async_poll(struct acquire_handle *handle) {
144145
}
145146
strncpy(handle->current_file, archive_entry_pathname(entry),
146147
sizeof(handle->current_file) - 1);
148+
handle->current_file[sizeof(handle->current_file) - 1] = '\0';
147149
handle->bytes_processed += archive_entry_size(entry);
148150
{
149151
char full_path[NAME_MAX * 2];

‎acquire/acquire_libcurl.h‎

Lines changed: 46 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,12 @@ int acquire_download_async_start(struct acquire_handle *handle, const char *url,
163163
CURL_SSLVERSION_TLSv1_2);
164164

165165
be->multi_handle = curl_multi_init();
166+
if (!be->multi_handle) {
167+
acquire_handle_set_error(handle, ACQUIRE_ERROR_NETWORK_INIT_FAILED,
168+
"curl_multi_init() failed");
169+
cleanup_curl_backend(handle);
170+
return -1;
171+
}
166172
curl_multi_add_handle(be->multi_handle, be->easy_handle);
167173

168174
handle->status = ACQUIRE_IN_PROGRESS;
@@ -171,51 +177,55 @@ int acquire_download_async_start(struct acquire_handle *handle, const char *url,
171177

172178
enum acquire_status acquire_download_async_poll(struct acquire_handle *handle) {
173179
struct curl_backend *be;
174-
int still_running = 0, r;
175-
CURLMsg *msg;
176-
if (!handle || !handle->backend_handle)
180+
CURLMcode mc;
181+
int still_running = 0;
182+
183+
/* 1. Basic sanity checks and state validation */
184+
if (handle == NULL)
177185
return ACQUIRE_ERROR;
178186
if (handle->status != ACQUIRE_IN_PROGRESS)
179187
return handle->status;
188+
if (handle->backend_handle == NULL) {
189+
acquire_handle_set_error(handle, ACQUIRE_ERROR_INVALID_ARGUMENT,
190+
"Polling on an uninitialized backend.");
191+
return ACQUIRE_ERROR;
192+
}
180193

181194
be = (struct curl_backend *)handle->backend_handle;
195+
196+
/* 2. Check for user cancellation first */
182197
if (handle->cancel_flag) {
183198
acquire_handle_set_error(handle, ACQUIRE_ERROR_CANCELLED,
184-
"Download cancelled by user");
185-
handle->status = ACQUIRE_ERROR;
199+
"Download cancelled by user.");
186200
cleanup_curl_backend(handle);
187-
return handle->status;
201+
return ACQUIRE_ERROR;
188202
}
189203

190-
r = curl_multi_perform(be->multi_handle, &still_running);
191-
if (r != CURLM_OK) {
204+
/* 3. Drive the multi stack to perform I/O */
205+
mc = curl_multi_perform(be->multi_handle, &still_running);
206+
if (mc != CURLM_OK) {
192207
acquire_handle_set_error(handle, ACQUIRE_ERROR_NETWORK_FAILURE,
193208
"curl_multi_perform() failed: %s",
194-
curl_multi_strerror(r));
209+
curl_multi_strerror(mc));
195210
cleanup_curl_backend(handle);
196-
return handle->status;
211+
return ACQUIRE_ERROR;
197212
}
198-
if (still_running == 0) {
199-
int queued;
200-
msg = curl_multi_info_read(be->multi_handle, &queued);
201-
if (msg) {
202-
if (msg->msg == CURLMSG_DONE) {
203-
long response_code = 0;
204213

205-
/* First, check if the transfer was successful */
214+
/* 4. Check for transfer completion messages */
215+
{
216+
CURLMsg *msg;
217+
int msgs_left;
218+
while ((msg = curl_multi_info_read(be->multi_handle, &msgs_left))) {
219+
if (msg->msg == CURLMSG_DONE) {
220+
/* This transfer is finished. Since we only manage one, the whole
221+
* operation is done. */
206222
if (msg->data.result == CURLE_OK) {
207-
curl_off_t cl;
208223
handle->status = ACQUIRE_COMPLETE;
209-
/* Explicitly get final size to ensure it's set */
210-
if (curl_easy_getinfo(msg->easy_handle,
211-
CURLINFO_CONTENT_LENGTH_DOWNLOAD_T,
212-
&cl) == CURLE_OK &&
213-
cl >= 0) {
214-
handle->total_size = (off_t)cl;
215-
}
216224
} else {
225+
long response_code = 0;
217226
curl_easy_getinfo(msg->easy_handle, CURLINFO_RESPONSE_CODE,
218227
&response_code);
228+
219229
if (msg->data.result == CURLE_COULDNT_RESOLVE_HOST) {
220230
acquire_handle_set_error(handle, ACQUIRE_ERROR_HOST_NOT_FOUND,
221231
"Could not resolve host: %s",
@@ -229,22 +239,21 @@ enum acquire_status acquire_download_async_poll(struct acquire_handle *handle) {
229239
curl_easy_strerror(msg->data.result));
230240
}
231241
}
242+
break; /* Exit loop, we only care about our one transfer */
232243
}
233-
} else {
234-
/* If still_running is 0 but we have no message, it implies success. */
235-
if (handle->status == ACQUIRE_IN_PROGRESS) {
236-
curl_off_t cl;
237-
handle->status = ACQUIRE_COMPLETE;
238-
if (curl_easy_getinfo(be->easy_handle,
239-
CURLINFO_CONTENT_LENGTH_DOWNLOAD_T,
240-
&cl) == CURLE_OK &&
241-
cl >= 0) {
242-
handle->total_size = (off_t)cl;
243-
}
244-
}
244+
}
245+
}
246+
247+
/* 5. If it's not running, the operation is over */
248+
if (still_running == 0) {
249+
if (handle->status == ACQUIRE_IN_PROGRESS) {
250+
/* If curl reports not running but we haven't received a DONE message,
251+
* it implies success. */
252+
handle->status = ACQUIRE_COMPLETE;
245253
}
246254
cleanup_curl_backend(handle);
247255
}
256+
248257
return handle->status;
249258
}
250259

‎acquire/acquire_libfetch.h‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,8 @@ int acquire_download_sync(struct acquire_handle *handle, const char *url,
4040

4141
u = fetchParseURL(url);
4242
if (u == NULL) {
43-
strncpy(handle->error_message, fetchLastErrString,
44-
sizeof(handle->error_message) - 1);
43+
acquire_handle_set_error(handle, ACQUIRE_ERROR_URL_PARSE_FAILED,
44+
fetchLastErrString);
4545
return -1;
4646
}
4747

@@ -52,23 +52,25 @@ int acquire_download_sync(struct acquire_handle *handle, const char *url,
5252

5353
f = fetchGet(u, "");
5454
if (f == NULL) {
55-
strncpy(handle->error_message, fetchLastErrString,
56-
sizeof(handle->error_message) - 1);
55+
acquire_handle_set_error(handle, ACQUIRE_ERROR_URL_PARSE_FAILED,
56+
fetchLastErrString);
5757
fetchFreeURL(u);
5858
return -1;
5959
}
6060

6161
handle->output_file = fopen(dest_path, "wb");
6262
if (!handle->output_file) {
63-
strcpy(handle->error_message, "Failed to open destination file");
63+
acquire_handle_set_error(handle, ACQUIRE_ERROR_FILE_OPEN_FAILED,
64+
"Failed to open destination file");
6465
fclose(f);
6566
fetchFreeURL(u);
6667
return -1;
6768
}
6869

6970
while ((bytes_read = fread(buffer, 1, sizeof(buffer), f)) > 0) {
7071
if (handle->cancel_flag) {
71-
strcpy(handle->error_message, "Download cancelled");
72+
acquire_handle_set_error(handle, ACQUIRE_ERROR_CANCELLED,
73+
"Download cancelled");
7274
break;
7375
}
7476
fwrite(buffer, 1, bytes_read, handle->output_file);

‎acquire/acquire_librhash.h‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,6 @@ int _librhash_verify_async_start(struct acquire_handle *handle,
106106
{
107107
const errno_t err = fopen_s(&be->file, filepath, "rb");
108108
if (err != 0 || be->file == NULL) {
109-
fprintf(stderr, "couldn't open file for reading %s\n", filepath);
110109
acquire_handle_set_error(handle, ACQUIRE_ERROR_FILE_OPEN_FAILED,
111110
"Cannot open file: %s", filepath);
112111
free(be);
@@ -116,7 +115,6 @@ int _librhash_verify_async_start(struct acquire_handle *handle,
116115
#else
117116
be->file = fopen(filepath, "rb");
118117
if (!be->file) {
119-
fprintf(stderr, "couldn't open file for reading %s\n", filepath);
120118
acquire_handle_set_error(handle, ACQUIRE_ERROR_FILE_OPEN_FAILED,
121119
"Cannot open file: %s", strerror(errno));
122120
free(be);
@@ -141,6 +139,7 @@ int _librhash_verify_async_start(struct acquire_handle *handle,
141139
}
142140
#else
143141
strncpy(be->expected_hash, expected_hash, sizeof(be->expected_hash) - 1);
142+
be->expected_hash[sizeof(be->expected_hash) - 1] = '\0';
144143
#endif
145144
handle->backend_handle = be;
146145
handle->status = ACQUIRE_IN_PROGRESS;

‎acquire/acquire_miniz.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
static int on_extract_entry(const char *filename, void *arg) {
1616
struct acquire_handle *handle = (struct acquire_handle *)arg;
1717
strncpy(handle->current_file, filename, sizeof(handle->current_file) - 1);
18+
handle->current_file[sizeof(handle->current_file) - 1] = '\0';
1819
return handle->cancel_flag ? -1 : 0;
1920
}
2021

‎acquire/acquire_openssl.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,6 @@ int _openssl_verify_async_start(struct acquire_handle *handle,
108108
{
109109
const errno_t err = fopen_s(&be->file, filepath, "rb");
110110
if (err != 0 || be->file == NULL) {
111-
fprintf(stderr, "couldn't open file for reading %s\n", filepath);
112111
acquire_handle_set_error(handle, ACQUIRE_ERROR_FILE_OPEN_FAILED,
113112
"Cannot open file: %s", filepath);
114113
free(be);
@@ -161,6 +160,7 @@ int _openssl_verify_async_start(struct acquire_handle *handle,
161160
}
162161
#else
163162
strncpy(be->expected_hash, expected_hash, sizeof(be->expected_hash) - 1);
163+
be->expected_hash[sizeof(be->expected_hash) - 1] = '\0';
164164
#endif
165165
handle->backend_handle = be;
166166
handle->status = ACQUIRE_IN_PROGRESS;

‎acquire/acquire_wincrypt.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,6 @@ int _wincrypt_verify_async_start(struct acquire_handle *handle,
100100
{
101101
const errno_t err = fopen_s(&be->file, filepath, "rb");
102102
if (err != 0 || be->file == NULL) {
103-
fprintf(stderr, "couldn't open file for reading %s\n", filepath);
104103
acquire_handle_set_error(handle, ACQUIRE_ERROR_FILE_OPEN_FAILED,
105104
"Cannot open file: %s", filepath);
106105
free(be);
@@ -126,6 +125,7 @@ int _wincrypt_verify_async_start(struct acquire_handle *handle,
126125
}
127126
#else
128127
strncpy(be->expected_hash, expected_hash, sizeof(be->expected_hash) - 1);
128+
be->expected_hash[sizeof(be->expected_hash) - 1] = '\0';
129129
#endif
130130
handle->backend_handle = be;
131131
handle->status = ACQUIRE_IN_PROGRESS;

‎acquire/acquire_wininet.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,6 @@ int acquire_download_sync(struct acquire_handle *handle, const char *url,
6666
{
6767
const errno_t err = fopen_s(&handle->output_file, dest_path, "wb");
6868
if (err != 0 || handle->output_file == NULL) {
69-
fprintf(stderr, "couldn't open file for reading %s\n", dest_path);
7069
acquire_handle_set_error(handle, ACQUIRE_ERROR_FILE_OPEN_FAILED,
7170
"Failed to open destination file: %s",
7271
dest_path);

‎acquire/tests/CMakeLists.txt‎

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -241,29 +241,18 @@ if (LIBACQUIRE_USE_LIBRHASH)
241241
endif (LIBACQUIRE_USE_LIBRHASH)
242242

243243
if (LIBACQUIRE_USE_LIBCURL)
244-
set(EXEC_NAME "test_curl_helpers")
244+
set(EXEC_NAME "test_libcurl_backend")
245245

246-
set(Test_Libcurl_Header_Files "test_libcurl_backend.h")
246+
set(Test_Libcurl_Header_Files "${EXEC_NAME}.h")
247247
source_group("${EXEC_NAME} Header Files" FILES "${Test_Libcurl_Header_Files}")
248248

249-
set(Test_Libcurl_Source_Files
250-
"test_libcurl_backend.c"
251-
"${CMAKE_BINARY_DIR}/gen/gen_acquire_libcurl.c"
252-
"${CMAKE_BINARY_DIR}/gen/gen_acquire_string_extras.c"
253-
)
254-
set_source_files_properties("${CMAKE_BINARY_DIR}/gen/gen_acquire_string_extras.c"
255-
PROPERTIES COMPILE_DEFINITIONS "LIBACQUIRE_IMPLEMENTATION=1;STRCASESTR_IMPL=1;STRNCASECMP_IMPL=1")
249+
set(Test_Libcurl_Source_Files "${EXEC_NAME}.c")
256250
source_group("${EXEC_NAME} Source Files" FILES "${Test_Libcurl_Source_Files}")
257251

258252
add_executable("${EXEC_NAME}" "${Test_Libcurl_Header_Files}" "${Test_Libcurl_Source_Files}")
259253

260254
target_compile_definitions(${EXEC_NAME} PRIVATE ACQUIRE_TESTING=1)
261255

262-
target_link_libraries("${EXEC_NAME}" PRIVATE CURL::libcurl)
263-
if (HAS_LIBBSD)
264-
target_link_libraries("${EXEC_NAME}" PRIVATE LibBSD::LibBSD)
265-
endif (HAS_LIBBSD)
266-
267256
target_link_libraries("${EXEC_NAME}" PRIVATE "${PROJECT_NAME}")
268257

269258
test_wrapper()

0 commit comments

Comments
 (0)