| Message ID | 1509647003-21020-1-git-send-email-philipp@redfish-solutions.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [LEDE-DEV,v1,1/1] uclient-fetch: correct filename w/ multple URLs | expand |
On 2017-11-02 19:23, Philip Prindeville wrote: > From: Philip Prindeville <philipp@redfish-solutions.com> > > When uclient-fetch is called with multiple URL's, it derives the > first filename based on the URL. When it then handles the 2nd and > subsequent URLs, it assumes that it was called with a -O filename > argument as the output file, because it tries to overload the > variable output_file to mean 2 different things. > > The fix is to use a bool to remember whether we were called with > an explicit output filename, i.e. with the -O argument, and not > overload output_file for this purpose. > > Signed-off-by: Philip Prindeville <philipp@redfish-solutions.com> Thanks for debugging this. I've decided to fix this with a different approach instead. I've pushed a change that avoids the overloading issue entirely by renaming the global variable and overwriting a local variable only. - Felix
> On Nov 2, 2017, at 3:03 PM, Felix Fietkau <nbd@nbd.name> wrote: > > On 2017-11-02 19:23, Philip Prindeville wrote: >> From: Philip Prindeville <philipp@redfish-solutions.com> >> >> When uclient-fetch is called with multiple URL's, it derives the >> first filename based on the URL. When it then handles the 2nd and >> subsequent URLs, it assumes that it was called with a -O filename >> argument as the output file, because it tries to overload the >> variable output_file to mean 2 different things. >> >> The fix is to use a bool to remember whether we were called with >> an explicit output filename, i.e. with the -O argument, and not >> overload output_file for this purpose. >> >> Signed-off-by: Philip Prindeville <philipp@redfish-solutions.com> > Thanks for debugging this. I've decided to fix this with a different > approach instead. I've pushed a change that avoids the overloading issue > entirely by renaming the global variable and overwriting a local > variable only. > > - Felix Okay, great. When will this show up in LEDE? Thanks.
On 2017-11-02 22:50, Philip Prindeville wrote: > >> On Nov 2, 2017, at 3:03 PM, Felix Fietkau <nbd@nbd.name> wrote: >> >> On 2017-11-02 19:23, Philip Prindeville wrote: >>> From: Philip Prindeville <philipp@redfish-solutions.com> >>> >>> When uclient-fetch is called with multiple URL's, it derives the >>> first filename based on the URL. When it then handles the 2nd and >>> subsequent URLs, it assumes that it was called with a -O filename >>> argument as the output file, because it tries to overload the >>> variable output_file to mean 2 different things. >>> >>> The fix is to use a bool to remember whether we were called with >>> an explicit output filename, i.e. with the -O argument, and not >>> overload output_file for this purpose. >>> >>> Signed-off-by: Philip Prindeville <philipp@redfish-solutions.com> >> Thanks for debugging this. I've decided to fix this with a different >> approach instead. I've pushed a change that avoids the overloading issue >> entirely by renaming the global variable and overwriting a local >> variable only. >> >> - Felix > > > Okay, great. > > When will this show up in LEDE? Pushed just now. - Felix
> On Nov 2, 2017, at 3:54 PM, Felix Fietkau <nbd@nbd.name> wrote: > > On 2017-11-02 22:50, Philip Prindeville wrote: >> >> Okay, great. >> >> When will this show up in LEDE? > Pushed just now. > > - Felix Tested and confirmed!
diff --git a/uclient-fetch.c b/uclient-fetch.c index dff144b22b7b3cd2d5982a615b9c2d68deab5042..fb3ab45935d94dce4f40aeae9bea6d7f7edc1c37 100644 --- a/uclient-fetch.c +++ b/uclient-fetch.c @@ -51,6 +51,7 @@ static bool proxy = true; static bool default_certs = false; static bool no_output; static const char *output_file; +static bool saw_output_filename = false; static int output_fd = -1; static int error_ret; static off_t out_offset; @@ -106,12 +107,12 @@ static int open_output_file(const char *path, uint64_t resume_offset) else flags = O_WRONLY | O_TRUNC; - if (!cur_resume && !output_file) + if (!cur_resume && !saw_output_filename) flags |= O_EXCL; flags |= O_CREAT; - if (output_file) { + if (saw_output_filename) { if (!strcmp(output_file, "-")) { if (!quiet) fprintf(stderr, "Writing to stdout\n"); @@ -500,6 +501,16 @@ static int no_ssl(const char *progname) return 1; } +static int too_many_output_files(const char *progname) +{ + fprintf(stderr, + "%s: the -O output_file option can't be used for multiple " + "URLs unless its value is \"-\".\n", + progname); + + return 1; +} + enum { L_NO_CHECK_CERTIFICATE, L_CA_CERTIFICATE, @@ -616,6 +627,7 @@ int main(int argc, char **argv) break; case 'O': output_file = optarg; + saw_output_filename = true; break; case 'P': if (chdir(optarg)) { @@ -651,6 +663,12 @@ int main(int argc, char **argv) if (argc < 1) return usage(progname); + /* doesn't make sense to use -O with multiple URL's unless you're + * sending them all to stdout... + */ + if (argc > 1 && saw_output_filename && strcmp(output_file, "-")) + return too_many_output_files(progname); + if (!ssl_ctx) { for (i = 0; i < argc; i++) { if (!strncmp(argv[i], "https", 5))