On Sun, Sep 03, 2006 at 10:25:52PM -0700, Ross Boylan <ross@betterworld.us> was heard to say:
> On Sun, Sep 03, 2006 at 09:50:22PM -0700, Daniel Burrows wrote:
> > On Sun, Sep 03, 2006 at 07:41:42PM -0700, Ross Boylan <ross@betterworld.us> was heard to say:
> > > So the second line has overwritten the buffer ("line"), which still
> > > holds the tail of the first line's message. It looks as if the
> > > problem is that 1) the error is multiline and 2) it is not in the
> > > multiline format that the work-around dealt with.
>
> It's peculiar that it's not expected, because 374195 reports on the
> same "very bad inconsistent state" error I encountered. At a guess,
> the different number of characters in the package name is causing
> things to line up badly (e.g., the double \000 in line), leading to
> the null value.
>
> >
> > I believe the problem here is that the code assumes that all strings
> > contain a colon character (':'). If a line does *not* contain a colon
> > character, the first entry of "list" is set to the whole line, and the
> > second entry is set to NULL to mark the end of the split list.
> >
> My different take was that the if(!TokSplitString...) test was
> expecting a false value if there weren't exactly as many tokens as
> expected. TokSplitString only returns false if there are too many
> fields. Also, note that TokSplitString scribbles \0's into the
> original line at the delimeters.
\0s are not NULL pointers. TokSplitString replaces delimiters in the
original string with \0, then writes pointers into the original string into
list. When it encounters the end of the original string, it writes a NULL
pointer to indicate the end of the list.
The third parameter to TokSplitString is the length of the list of char*s
it gets passed. If there isn't enough room in the list for all the tokens
it found, it clobbers the last token (by writing NULL over it), then returns
false.
> At any rate, I came up with a slightly different patch. With this
> patch, the aptitude operation fails to work without crashing. In
> other words, it seems to solve the problem. However, I haven't even
> verified that aptitude continues to function under normal conditions
> with this patch.
>
> --- apt-0.6.45/apt-pkg/deb/dpkgpm.cc 2006-07-25 04:19:00.000000000 -0700
> +++ apt-0.6.45rb/apt-pkg/deb/dpkgpm.cc 2006-09-03 21:33:07.000000000 -0700
> @@ -632,4 +632,13 @@
> // it. for now we just ignore it and not crash
> continue;
> +
> + /* The previous code only becomes active if there are more than
> + the expected number of : separated fields.
> + I think the next test may match intent better, but it may
> + be too broad, and it may not work in all locales.
> + Ross Boylan 2006-09-03 */
> + if(strncmp(list[0],"status",strlen("status")) != 0)
> + continue;
> +
> char *pkg = list[1];
> char *action = _strstrip(list[2]);
That will still allow crashes if you happen to get a second line
containing only the word "status". Yes, it's unlikely, but why not
solve the crash directly and not have to guess?
Daniel
Attachment:
signature.asc
Description: Digital signature