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

diff --git a/src/tool_cfgable.h b/src/tool_cfgable.h
index 8c1dd8d269d65cf3639cac179032647686f262d6..80411f2cd85046303c1b59e2ef3d3888a01f54ab 100644 (file)
--- 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 e8dac9691ddb3529bbf55eb0c63f267f314299c6..1158d857363b2356a601f8a1dbc1d07f6b8c8ac2 100644 (file)
--- 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 93904bbe16e4c5da903b9ab4be89650273453b53..48129973f98732ecf0734da183270c554d62ec0c 100644 (file)
--- 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)