Re: Fwd: [bug-inetutils] [PATCH] fix whitespace parsing in syslogd

[Julian Gilbey]

On Mon, Sep 01, 2003 at 02:35:42PM +0200, Marcus Brinkmann wrote:
> On Mon, Sep 01, 2003 at 11:36:08AM +0100, Julian Gilbey wrote:
> > On Fri, Aug 29, 2003 at 04:13:11PM +0000, Robert Millan wrote:
> > > 
> > > Hi Julian,
> > > 
> > > Upstream revised your patch and provides a new version (attached). Please
> > > could you tell us wether this one fixes it?
> > 
> > This would appear to fix the continuation line part; however, the
> 
> "would" it fix it, or does it fix it?  Can you please test that it works
> correctly if you have not done so already?

Applied; works correctly.

> > patch I submitted also fixed another part of the code, which dealt
> > with the input line being too long.  I wouldn't want to see that lost,
> > either.
> 
> The bug report contained no information about that, and you didn't write a
> changelog for your patch.  So if there is another bug beside the problematic
> whitespace, we will need another report for that and we can take a look at
> it.

Sorry, here goes.  (Not exactly sure what you mean by "we will need
another report, though.)

The original patch contains the following extra bits (now modified
because of your improved patch); comments and explanation follow:

--- syslogd/syslogd.c.orig      2003-08-23 22:34:40.000000000 +0100
+++ syslogd/syslogd.c   2003-08-24 00:39:27.000000000 +0100
@@ -1679,7 +1679,7 @@
   cbuf = malloc (line_max);
   if (cbuf == NULL)
     {
-      /* There is no gracefull recovery here.  */
+      /* There is no graceful recovery here.  */
       dbg_printf ("cannot allocate space for configuration\n");
       return;
     }
@@ -1699,6 +1699,7 @@
       /* No newline ? then line is too big for the buffer readjust.  */
       if (memchr (cline, '\n', len) == NULL)
        {
+         size_t offset = cline - cbuf;
          char *tmp;
          tmp = realloc (cbuf, line_max * 2);
          if (tmp == NULL)
@@ -1711,9 +1712,8 @@
          else
            cbuf = tmp;
          line_max *= 2;
-         strcpy (cbuf, cline);
-         cline += len;
+         cline = cbuf + offset + len - 1;  /* reset cline for new cbuf */
          continue;
        }
       else
        cline = cbuf;


The first patch snippet is just a typo in a comment I happened to
notice ;-)

The rest of the patch is more subtle: it is in the section of the code
which deals with what happens if a configuration file line is too
long.  What happened in the original is that cbuf was realloc'd in
such a case, but then line 1714 does a meaningless strcpy from cline -
which is a pointer into the *old* cbuf - into the new cbuf, which is
unnecessary as realloc guarantees to copy the old cbuf into the newly
allocated memory.  So instead, what is needed is simply for cline to
be set the the correct place in the new cbuf, and this patch does this
task.  (Note that offset needs to be calculated *before* the realloc!)

   Julian

-- 
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-

        Julian Gilbey, website: http://www.polya.uklinux.net/
   Debian GNU/Linux Developer, see: http://people.debian.org/~jdg/
     Visit http://www.thehungersite.com/ to help feed the hungry