From b2ccfbf2fbdf1c22abf6c43882d1d5b84ab88bbf Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Tue, 29 Jul 2025 13:29:25 +0200 Subject: [PATCH] tool_operate: simplify single_transfer - let the caller do the cleanup on fail - avoid gotos and use direct returns more - use while() loop Closes #18077 --- src/tool_cfgable.h | 2 +- src/tool_operate.c | 195 +++++++++++++++++++-------------------------- src/tool_operhlp.c | 2 +- 3 files changed, 85 insertions(+), 114 deletions(-) diff --git a/src/tool_cfgable.h b/src/tool_cfgable.h index 8c1dd8d269..80411f2cd8 100644 --- a/src/tool_cfgable.h +++ b/src/tool_cfgable.h @@ -74,7 +74,7 @@ struct State { curl_off_t infilenum; /* number of files to upload */ curl_off_t up; /* upload file counter within a single upload glob */ curl_off_t urlnum; /* how many iterations this URL has with ranges etc */ - curl_off_t li; + curl_off_t li; /* index for globbed URLs */ }; struct OperationConfig { diff --git a/src/tool_operate.c b/src/tool_operate.c index e8dac9691d..1158d85736 100644 --- a/src/tool_operate.c +++ b/src/tool_operate.c @@ -347,15 +347,16 @@ static CURLcode pre_transfer(struct GlobalConfig *global, void single_transfer_cleanup(struct OperationConfig *config) { - if(config) { - struct State *state = &config->state; - /* Free list of remaining URLs */ - glob_cleanup(&state->urls); - state->outfiles = NULL; - tool_safefree(state->uploadfile); - /* Free list of globbed upload files */ - glob_cleanup(&state->inglob); - } + struct State *state; + DEBUGASSERT(config); + + state = &config->state; + /* Free list of remaining URLs */ + glob_cleanup(&state->urls); + state->outfiles = NULL; + tool_safefree(state->uploadfile); + /* Free list of globbed upload files */ + glob_cleanup(&state->inglob); } static CURLcode retrycheck(struct OperationConfig *config, @@ -1110,7 +1111,6 @@ static CURLcode single_transfer(struct OperationConfig *config, bool *skipped) { CURLcode result = CURLE_OK; - struct getout *urlnode; struct GlobalConfig *global = config->global; bool orig_noprogress = global->noprogress; bool orig_isatty = global->isatty; @@ -1126,63 +1126,49 @@ static CURLcode single_transfer(struct OperationConfig *config, httpgetfields = state->httpgetfields = config->postfields; config->postfields = NULL; if(SetHTTPrequest(config, (config->no_body ? TOOL_HTTPREQ_HEAD : - TOOL_HTTPREQ_GET), &config->httpreq)) { - result = CURLE_FAILED_INIT; - } + TOOL_HTTPREQ_GET), &config->httpreq)) + return CURLE_FAILED_INIT; } } - else { - if(SetHTTPrequest(config, TOOL_HTTPREQ_SIMPLEPOST, &config->httpreq)) - result = CURLE_FAILED_INIT; - } - if(result) - goto fail; + else if(SetHTTPrequest(config, TOOL_HTTPREQ_SIMPLEPOST, &config->httpreq)) + return CURLE_FAILED_INIT; } + + result = set_cert_types(config); + if(result) + return result; + if(!state->urlnode) { /* first time caller, setup things */ state->urlnode = config->url_list; state->infilenum = 1; } - result = set_cert_types(config); - if(result) - goto fail; - - for(; state->urlnode; state->urlnode = urlnode->next) { - static bool warn_more_options = FALSE; - curl_off_t urlnum; - - urlnode = state->urlnode; - /* urlnode->url is the full URL or NULL */ - if(!urlnode->url) { - /* This node has no URL. Free node data without destroying the - node itself nor modifying next pointer and continue to next */ - urlnode->outset = urlnode->urlset = urlnode->useremote = - urlnode->uploadset = urlnode->noupload = urlnode->noglob = FALSE; - state->up = 0; - if(!warn_more_options) { - /* only show this once */ - warnf(config->global, "Got more output options than URLs"); - warn_more_options = TRUE; - } - continue; /* next URL please */ + while(state->urlnode) { + struct getout *u = state->urlnode; + + /* u->url is the full URL or NULL */ + if(!u->url) { + /* This node has no URL. End of the road. */ + warnf(config->global, "Got more output options than URLs"); + break; } /* save outfile pattern before expansion */ - if(urlnode->outfile && !state->outfiles) - state->outfiles = urlnode->outfile; + if(u->outfile && !state->outfiles) + state->outfiles = u->outfile; - if(!config->globoff && urlnode->infile && !state->inglob) { + if(!config->globoff && u->infile && !state->inglob) { /* Unless explicitly shut off */ - result = glob_url(&state->inglob, urlnode->infile, &state->infilenum, + result = glob_url(&state->inglob, u->infile, &state->infilenum, (!global->silent || global->showerror) ? tool_stderr : NULL); if(result) - break; + return result; } - if(state->up || urlnode->infile) { + if(state->up || u->infile) { if(!state->uploadfile) { if(state->inglob) { result = glob_next_url(&state->uploadfile, state->inglob); @@ -1191,30 +1177,27 @@ static CURLcode single_transfer(struct OperationConfig *config, } else if(!state->up) { /* copy the allocated string */ - state->uploadfile = urlnode->infile; - urlnode->infile = NULL; + state->uploadfile = u->infile; + u->infile = NULL; } } if(result) - break; + return result; } if(!state->urlnum) { - if(!config->globoff && !urlnode->noglob) { + if(!config->globoff && !u->noglob) { /* Unless explicitly shut off, we expand '{...}' and '[...]' expressions and return total number of URLs in pattern set */ - result = glob_url(&state->urls, urlnode->url, &state->urlnum, + result = glob_url(&state->urls, u->url, &state->urlnum, (!global->silent || global->showerror) ? tool_stderr : NULL); if(result) - break; - urlnum = state->urlnum; + return result; } else - urlnum = 1; /* without globbing, this is a single URL */ + state->urlnum = 1; /* without globbing, this is a single URL */ } - else - urlnum = state->urlnum; if(state->up < state->infilenum) { struct per_transfer *per = NULL; @@ -1234,7 +1217,7 @@ static CURLcode single_transfer(struct OperationConfig *config, if(config->etag_compare_file) { result = etag_compare(config); if(result) - break; + return result; } if(config->etag_save_file) { @@ -1253,27 +1236,21 @@ static CURLcode single_transfer(struct OperationConfig *config, curl_easy_cleanup(curl); if(etag_save->fopened) fclose(etag_save->stream); - break; + return result; } per->etag_save = etag_first; /* copy the whole struct */ if(state->uploadfile) { per->uploadfile = strdup(state->uploadfile); - if(!per->uploadfile) { - curl_easy_cleanup(curl); - result = CURLE_OUT_OF_MEMORY; - break; - } - if(SetHTTPrequest(config, TOOL_HTTPREQ_PUT, &config->httpreq)) { + if(!per->uploadfile || + SetHTTPrequest(config, TOOL_HTTPREQ_PUT, &config->httpreq)) { tool_safefree(per->uploadfile); curl_easy_cleanup(curl); - result = CURLE_FAILED_INIT; - break; + return CURLE_FAILED_INIT; } } - *added = TRUE; per->config = config; per->curl = curl; - per->urlnum = (unsigned int)urlnode->num; + per->urlnum = (unsigned int)u->num; /* default headers output stream is stdout */ heads = &per->heads; @@ -1283,7 +1260,7 @@ static CURLcode single_transfer(struct OperationConfig *config, if(config->headerfile) { result = setup_headerfile(config, per, heads); if(result) - break; + return result; } hdrcbdata = &per->hdrcbdata; @@ -1299,34 +1276,30 @@ static CURLcode single_transfer(struct OperationConfig *config, if(state->urls) { result = glob_next_url(&per->url, state->urls); if(result) - break; + return result; } else if(!state->li) { - per->url = strdup(urlnode->url); - if(!per->url) { - result = CURLE_OUT_OF_MEMORY; - break; - } + per->url = strdup(u->url); + if(!per->url) + return CURLE_OUT_OF_MEMORY; } - else + else { per->url = NULL; - if(!per->url) break; + } if(state->outfiles) { per->outfile = strdup(state->outfiles); - if(!per->outfile) { - result = CURLE_OUT_OF_MEMORY; - break; - } + if(!per->outfile) + return CURLE_OUT_OF_MEMORY; } - outs->out_null = urlnode->out_null; - if(!outs->out_null && (urlnode->useremote || + outs->out_null = u->out_null; + if(!outs->out_null && (u->useremote || (per->outfile && strcmp("-", per->outfile)))) { result = setup_outfile(config, per, outs, skipped); if(result) - break; + return result; } if(per->uploadfile) { @@ -1340,7 +1313,7 @@ static CURLcode single_transfer(struct OperationConfig *config, result = add_file_name_to_url(per->curl, &per->url, per->uploadfile); if(result) - break; + return result; } } @@ -1363,7 +1336,7 @@ static CURLcode single_transfer(struct OperationConfig *config, result = append2query(config, per, httpgetfields ? httpgetfields : config->query); if(result) - break; + return result; } if((!per->outfile || !strcmp(per->outfile, "-")) && @@ -1377,7 +1350,7 @@ static CURLcode single_transfer(struct OperationConfig *config, config->terminal_binary_ok = (per->outfile && !strcmp(per->outfile, "-")); - if(config->content_disposition && urlnode->useremote) + if(config->content_disposition && u->useremote) hdrcbdata->honor_cd_filename = TRUE; else hdrcbdata->honor_cd_filename = FALSE; @@ -1390,7 +1363,7 @@ static CURLcode single_transfer(struct OperationConfig *config, result = config2setopts(config, per, curl, share); if(result) - break; + return result; /* initialize retry vars for loop below */ per->retry_sleep_default = (config->retry_delay) ? @@ -1401,37 +1374,32 @@ static CURLcode single_transfer(struct OperationConfig *config, state->li++; /* Here's looping around each globbed URL */ - if(state->li >= urlnum) { + if(state->li >= state->urlnum) { state->li = 0; state->urlnum = 0; /* forced reglob of URLs */ glob_cleanup(&state->urls); state->up++; tool_safefree(state->uploadfile); /* clear it to get the next */ } + *added = TRUE; + break; } - else { - /* Free this URL node data without destroying the - node itself nor modifying next pointer. */ - urlnode->outset = urlnode->urlset = urlnode->useremote = - urlnode->uploadset = urlnode->noupload = urlnode->noglob = FALSE; - glob_cleanup(&state->urls); - state->urlnum = 0; - state->outfiles = NULL; - tool_safefree(state->uploadfile); - /* Free list of globbed upload files */ - glob_cleanup(&state->inglob); - state->up = 0; - continue; - } - break; + /* Free this URL node data without destroying the + node itself nor modifying next pointer. */ + u->outset = u->urlset = u->useremote = + u->uploadset = u->noupload = u->noglob = FALSE; + glob_cleanup(&state->urls); + state->urlnum = 0; + + state->outfiles = NULL; + tool_safefree(state->uploadfile); + /* Free list of globbed upload files */ + glob_cleanup(&state->inglob); + state->up = 0; + state->urlnode = u->next; /* next node */ } state->outfiles = NULL; -fail: - if(!*added || result) { - *added = FALSE; - single_transfer_cleanup(config); - } return result; } @@ -2033,7 +2001,7 @@ static CURLcode serial_transfers(struct GlobalConfig *global, /* returncode errors have priority */ result = returncode; - if(result) + if(result && global->current) single_transfer_cleanup(global->current); return result; @@ -2166,8 +2134,11 @@ static CURLcode transfer_per_config(struct OperationConfig *config, result = cacertpaths(config); } - if(!result) + if(!result) { result = single_transfer(config, share, added, skipped); + if(!*added || result) + single_transfer_cleanup(config); + } return result; } diff --git a/src/tool_operhlp.c b/src/tool_operhlp.c index 93904bbe16..48129973f9 100644 --- a/src/tool_operhlp.c +++ b/src/tool_operhlp.c @@ -45,8 +45,8 @@ void clean_getout(struct OperationConfig *config) node = next; } config->url_list = NULL; + single_transfer_cleanup(config); } - single_transfer_cleanup(config); } bool output_expected(const char *url, const char *uploadfile) -- 2.47.2