| Message ID | 20210126131412.3567-2-michael.adler@siemens.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Overhaul swupdate.cfg handling | expand |
Hi Michael, On 26.01.21 14:14, Michael Adler wrote: > The current behavior is to set the log level in the *second* > getopt_long(3) parsing stage of SWUpdate. In other words, all log > messages with a finer granularity (e.g. INFO) which are created *before* > reaching the second getopt parsing stage are dropped since the default > log level is ERROR. This is known because first stage getopt is just to get the filename for the config. > This makes it (unneccesarily) difficult to debug > issues in the swupdate.cfg parsing code. > To allow for config overloading, we continue to parse the loglevel a > second time after loading and parsing swupdate.cfg. > > Signed-off-by: Michael Adler <michael.adler@siemens.com> > Signed-off-by: Christian Storm <christian.storm@siemens.com> > --- > core/swupdate.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/core/swupdate.c b/core/swupdate.c > index 4f008e0..788981e 100644 > --- a/core/swupdate.c > +++ b/core/swupdate.c > @@ -516,6 +516,12 @@ int main(int argc, char **argv) > case 'f': > cfgfname = sdup(optarg); > break; > + case 'l': > + loglevel = strtoul(optarg, NULL, 10); > + break; > + case 'v': > + loglevel = TRACELEVEL; > + break; > case '0': > printf("%s", BANNER); > exit(EXIT_SUCCESS); > Ok, so move these to the first getopt(). What about then the later, that is: 585 switch (c) { 586 case 'v': 587 loglevel = TRACELEVEL; 588 break; and 601 case 'l': 602 loglevel = strtoul(optarg, NULL, 10); 603 break; They are then duplicated, aren't they ? Best regards, Stefano
Hi Stefano,
> They are then duplicated, aren't they ?
yes, it is duplicated but I think for a valid reason, as stated in the (already existing) code:
/*
* Command line should be parsed a second time
* This let line parameters overload
* [...]
*/
For instance, if swupdate.cfg contains loglevel "info", and `swupdate -v` is started, the following will happen:
1) the first getopt_long block is executed and loglevel is set to TRACE (because of the -v flag)
2) swupdate.cfg is parsed, loglevel is set to INFO
3) getopt_long is executed again, loglevel is set to TRACE (again)
So CLI takes precedence over config file, as desired.
However, I agree that it feels a bit 'unclean' to mutate the log level like this: some log messages which are created
between stages 2) and 3) might be lost.
If you agree with me, I could provide a patch to change the log level in 2) only if it was not set in 1).
Kind regards,
Michael
diff --git a/core/swupdate.c b/core/swupdate.c index 4f008e0..788981e 100644 --- a/core/swupdate.c +++ b/core/swupdate.c @@ -516,6 +516,12 @@ int main(int argc, char **argv) case 'f': cfgfname = sdup(optarg); break; + case 'l': + loglevel = strtoul(optarg, NULL, 10); + break; + case 'v': + loglevel = TRACELEVEL; + break; case '0': printf("%s", BANNER); exit(EXIT_SUCCESS);