Luca - Here's the same thing standardizing on strn?cmp(). Not that you couldn't have done it yourself, but since I had it up, maybe this will save you 30 seconds. ;-)
Index: server/core.c =================================================================== --- server/core.c (revision 1829439) +++ server/core.c (working copy) @@ -4867,7 +4867,8 @@ static int check_errorlog_dir(apr_pool_t *p, server_rec *s) { if (!s->error_fname || s->error_fname[0] == '|' - || strcmp(s->error_fname, "syslog") == 0) { + || strcmp(s->error_fname, "syslog") == 0 + || strncmp(s->error_fname, "syslog:", 7) == 0) { return APR_SUCCESS; } else { @@ -5281,7 +5282,9 @@ apr_file_printf(out, "ServerRoot: \"%s\"\n", ap_server_root); tmp = ap_server_root_relative(p, sconf->ap_document_root); apr_file_printf(out, "Main DocumentRoot: \"%s\"\n", tmp); - if (s->error_fname[0] != '|' && strcmp(s->error_fname, "syslog") != 0) + if (s->error_fname[0] != '|' + && strcmp(s->error_fname, "syslog") != 0 + && strncmp(s->error_fname, "syslog:", 7) != 0) tmp = ap_server_root_relative(p, s->error_fname); else tmp = s->error_fname; @@ -5456,4 +5459,3 @@ core_cmds, /* command apr_table_t */ register_hooks /* register hooks */ }; - Index: server/log.c =================================================================== --- server/log.c (revision 1829439) +++ server/log.c (working copy) @@ -396,7 +396,8 @@ } #ifdef HAVE_SYSLOG - else if (!strncasecmp(s->error_fname, "syslog", 6)) { + else if (strcmp(s->error_fname, "syslog") == 0 + || strncmp(s->error_fname, "syslog:", 7) == 0) { if ((fname = strchr(s->error_fname, ':'))) { /* s->error_fname could be [level]:[tag] (see #60525) */ const char *tag; > On 18 Apr 2018, at 13:32, Luca Toscano <toscano.l...@gmail.com> wrote: > > Thanks a lot Jim! I like your code change and the extra checks, but I'd > prefer to use strncmp if possible, also in log.c. > Feel free to amend the patch, or I'll do it tomorrow (I forgot the entry in > CHANGES too, your name should be on it :). > > Luca > > 2018-04-18 18:36 GMT+02:00 Jim Riggs <j...@riggs.me>: > Fair enough. I'm fine standardizing either way. strn?cmp() is probably more > "correct". As it stands, though, the check in core is not actually checking > everything that log.c will handle. > > > > On 18 Apr 2018, at 10:15, William A Rowe Jr <wr...@rowe-clan.net> wrote: > > > > On Wed, Apr 18, 2018 at 7:17 AM, Jim Riggs <jim...@riggs.me> wrote: > >> I didn't think of this before, but there is one edge case this would miss: > >> if someone (for whatever reason) wants a relative ErrorLog *file* named > >> `syslog*', for example `ErrorLog "syslog-httpd.log"' or `ErrorLog > >> "syslog.log"'. It appears that this is already broken in server/log.c, > >> though. Also, log.c is using str(n)casecmp. The following would make > >> things consistent and handle this weird edge case. Thoughts? > >> > >> Index: server/core.c > >> =================================================================== > >> --- server/core.c (revision 1829431) > >> +++ server/core.c (working copy) > >> @@ -4867,7 +4867,8 @@ > >> static int check_errorlog_dir(apr_pool_t *p, server_rec *s) > >> { > >> if (!s->error_fname || s->error_fname[0] == '|' > >> - || strcmp(s->error_fname, "syslog") == 0) { > >> + || strcasecmp(s->error_fname, "syslog") == 0 > >> + || strncasecmp(s->error_fname, "syslog:", 7) == 0) { > >> return APR_SUCCESS; > >> } > >> else { > >> @@ -5281,7 +5282,9 @@ > >> apr_file_printf(out, "ServerRoot: \"%s\"\n", ap_server_root); > >> tmp = ap_server_root_relative(p, sconf->ap_document_root); > >> apr_file_printf(out, "Main DocumentRoot: \"%s\"\n", tmp); > >> - if (s->error_fname[0] != '|' && strcmp(s->error_fname, "syslog") != 0) > >> + if (s->error_fname[0] != '|' > >> + && strcasecmp(s->error_fname, "syslog") != 0 > >> + && strncasecmp(s->error_fname, "syslog:", 7) != 0) > >> tmp = ap_server_root_relative(p, s->error_fname); > >> else > >> tmp = s->error_fname; > >> @@ -5456,4 +5459,3 @@ > >> core_cmds, /* command apr_table_t */ > >> register_hooks /* register hooks */ > >> }; > >> - > >> Index: server/log.c > >> =================================================================== > >> --- server/log.c (revision 1829431) > >> +++ server/log.c (working copy) > >> @@ -396,7 +396,8 @@ > >> } > >> > >> #ifdef HAVE_SYSLOG > >> - else if (!strncasecmp(s->error_fname, "syslog", 6)) { > >> + else if (strcasecmp(s->error_fname, "syslog") == 0 > >> + || strncasecmp(s->error_fname, "syslog:", 7) == 0) { > >> if ((fname = strchr(s->error_fname, ':'))) { > >> /* s->error_fname could be [level]:[tag] (see #60525) */ > >> const char *tag; > > > > Shouldn't we normalize the use of strcmp instead of strcasecmp? > > In any case it must be entirely normalized to one or the other. > > > > Linux is a case-sensitive OS in the first place, and if configured > > with SYSLOG: today it is broken when you hit core, which ignores > > that code path. Since the behavior has always been syslog: I'm > > not seeing a benefit to case insensitivity. > >