option parsing for --log still broken

  Hi Klaus and all, 

it appears that option parsing for the --log option is still broken, as
described in this thread: https://www.vdr-portal.de/forum/index.php?thread/108924-gel%C3%B6st-logging-von-yavdr-in-separaten-logfiles-anstelle-des-syslog-und-probleme/

Short version: vdr stops parsing the remainder of the command line at the
point of --log=2.6. 

Long version: 

The problem is in the getopt code in vdr.c: 

          case 'l': {
                    char *p = strchr(optarg, '.');
                    if (p)
                       *p = 0;
                    if (isnumber(optarg)) {
                       int l = atoi(optarg);
                       if (0 <= l && l <= 3) {
                          SysLogLevel = l;
                          if (!p)
                             break;
                          if (isnumber(p + 1)) {
                             int l = atoi(p + 1);
                             if (0 <= l && l <= 7) {
                                int targets[] = { LOG_LOCAL0, LOG_LOCAL1, LOG_LOCAL2, LOG_LOCAL3, LOG_LOCAL4, LOG_LOCAL5, LOG_LOCAL6, LOG_LOCAL7 };
                                SysLogTarget = targets[l];
                                break;
                                }
                             }
                          }
                       }
                    if (p)
                       *p = '.';
                    fprintf(stderr, "vdr: invalid log level: %s\n", optarg);
                    return 2;
                    }

Initially, the point is replaced by a NULL byte in order to be able to atoi()
both numbers (if (p) *p = 0;). This replacing takes place _in place_. In theory, the replace
should be reverted at the end of the code snippet (if (p) *p = '.';) BUT if
one of the branches involving a break is executed, the revert is never
invoked, and the original command line remains broken so that the remaining
command line options are not getting evaluated. Consequently, what used to be 

vdr -w 60 -g -u vdr -l 2.6 -P plugin1 -P plugin2

becomes 

vdr -w 60 -g -u vdr -l 2 6 -P plugin1 -P plugin2

and the modules will not get loaded. The thread mentioned above speaks of
upstart & co but I have the same issue in a docker container with explicit
program start in a shell script whithout any upstart or anything. I now helped
myself putting the -l at the end of the command line but I still cannot send
the vdr log to a different facility because the dangling 6 is not getting
evaluated. 

My suggestion would be to not evaluate the optarg string directly but create a copy
of optarg first, and work with the copy. This way, the original command line
is unchanged.  Sample patch attached, untested but it should do the trick.
  

On 17.11.20 12:17, Harald Milz wrote:
> Hi Klaus and all,
> 
> it appears that option parsing for the --log option is still broken, as
> described in this thread: https://www.vdr-portal.de/forum/index.php?thread/108924-gel%C3%B6st-logging-von-yavdr-in-separaten-logfiles-anstelle-des-syslog-und-probleme/
> ...

In your patch, if optarg is longer than 3 characters, strncpy() wouldn't terminate
'copy' with 0.

This should do, too:

--- vdr.c       2020/05/18 16:47:29     4.33
+++ vdr.c       2020/11/17 17:11:51
@@ -422,6 +422,7 @@
                            SysLogLevel = l;
                            if (!p)
                               break;
+                          *p = '.';
                            if (isnumber(p + 1)) {
                               int l = atoi(p + 1);
                               if (0 <= l && l <= 7) {

The first break doesn't matter, because p is NULL in that case and thus optarg has
not been changed.

Klaus
  

Hi Klaus, 

On Tue, Nov 17, 2020 at 06:15:51PM +0100, Klaus Schmidinger wrote:
> This should do, too:
> 
> --- vdr.c       2020/05/18 16:47:29     4.33
> +++ vdr.c       2020/11/17 17:11:51
> @@ -422,6 +422,7 @@
>                            SysLogLevel = l;
>                            if (!p)
>                               break;
> +                          *p = '.';

Yes, that's much better. Just tried it and I can confirm it works. The command
line in ps now looks like

/usr/bin/vdr -v /video ... --log=2.6 -P epgsearch ...

Before, the respective part looked like "--log=2 6" and everything after
it was ignored by vdr, ie plugins not loaded. 

And it logs to LOCAL6 just fine. 

Just building new Ubuntu focal vdr-2.4.1 package. I suggest to put this patch upstream because
it will cure a couple of people's headaches ;-) 

Thank you!