busybox current status

[Denis Vlasenko]

On Saturday 02 December 2006 22:48, Roberto A. Foglietta wrote:
> 2006/12/1, Denis Vlasenko <vda.linux at googlemail.com>:
> > On Thursday 30 November 2006 13:20, Roberto A. Foglietta wrote:
> >
> > > > To do:
> > > >
> > > > * more fishing in bug database
> > >
> > > the bug n.615 still open and I have updated patch to 1.2.2.1 and today
> > > snapshot (in attachment)
> > > http://bugs.busybox.net/view.php?id=615
> >
> > -               if (temp) {
> > -                       *no_newline = !(len && temp[len-1] == '\n');
> > +               if (temp && len > 0) {
> > +                       endzero += !temp[len-1];
> > +                       *no_newline = !(temp[len-1]=='\n');
> >                         if (!*no_newline) temp[len-1] = 0;
> >                         break;
> >
> > old code: if len == 0: 'break' is taken
> > new code: if len == 0: 'break' is not taken
> > is it ok?
> >
> 
> I tested your patch but it seems it does not work at least against
> with 2006-12-01

I sent you new sed.c which has that fixed.

Roberto, I was looking into your patch busybox-20061130_sed.patch.
I have some problems with it.

Try to limit usage of static data.

Try to make code easier to understand. sed code is convoluted enough
already, have heart for poor souls (probably you, a year from now)
which will come later and will try to figure out how it works.
It's best to try to make it so that C code itself is easy to read,
but if you have clever but obscure trick, document how it works
in comments. Reread your comments when you generate patch
for submission. I frequently find my own comments too cryptic
and edit them again.

About that particular patch: "static char prisubst" variable name
is not descriptive. Is it used to skip subst commands (if I read
the code right)? Why it is called prisubst and not skip_next_subst?
--
vda