BusyBox 1.11.0 man patch for "cat"

Denys Vlasenko vda.linux at googlemail.com
Fri Jul 4 21:17:43 UTC 2008


On Friday 04 July 2008 22:36, Jason Curl wrote:
> Denys Vlasenko wrote:
> > On Friday 04 July 2008 09:55, Jason Curl wrote:
> >   
> >> Hello,
> >>
> >> I'd like to welcome comments on my first bb patch
> >>     
> >
> > It's using spaces for indentation. Otherwise looks ok.
> >   
> OK, I'll fix this.
> >   
> >> (it's the first I've needed to do since I've started
> >> using 1.0.0). I like the new "man" applet, but it
> >> needs nroff and gtbl. Unfortunately, I didn't prepare
> >> the c++ compiler and the gnu tools won't compile.
> >> No worry though, I've updated busybox to support
> >> "cat" pages also. This requires less space than
> >> installing all the GNU tools.
> >>     
> >
> > Ideally, some simplified replacement for nroff+gtbl
> > would be cool.
> >   
> 
> I've seem to found a bug while testing man in a bit more detail. It 
> looks like the line
>     alloc_mp = 10;
>     /* this doesn't appear to initialise the array to NULL pointers */
>     man_path_list = xmalloc(10 * sizeof(man_path_list[0]));
>     count_mp = 0;
>     man_path_list[0] = xstrdup(getenv("MANPATH"));
> 
> doesn't initialise the array to zero in my case (indicating NULL 
> pointers). So as this array gets built up while parsing "/etc/man.conf" 
> it assumes that it was previously NULL.
>                 if (strcmp("MANPATH", line) == 0) {
>                     man_path_list[count_mp] = xstrdup(value);
>                     count_mp++;
>                     if (alloc_mp == count_mp) {
>                         alloc_mp += 10;
>                         man_path_list = xrealloc(man_path_list, alloc_mp 
> * sizeof(man_path_list[0]));
>                     }
>                     /* thus man_path_list is always NULL terminated */
> 
> That last comment doesn't seem to hold true.
> 
> Then the final while loop checks
>         while ((cur_path = man_path_list[cur_mp++]) != NULL) {
> 
> and it turns out that uninitialised data is compared against NULL which 
> fails, enters the loop again and dies with a segfault.
> 
> So the solution appears to be
>   if (cf) {
>     ..
>   }
>   man_path_list[count_mp] = NULL;
> 
> This fixes my segfault.

Plase try this patch. It has an advantage of also making code smaller.
--
vda

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 6.patch
Type: text/x-diff
Size: 1608 bytes
Desc: not available
Url : http://lists.busybox.net/pipermail/busybox/attachments/20080704/779d6c49/attachment.bin 


More information about the busybox mailing list