Michael Tokarev:
> 15.03.2025 19:32, Wietse Venema via Postfix-devel ?????:
> > Michael Tokarev via Postfix-devel:
> >> There's an interesting twist in vstring_vstream.c, when
> >> two typos, each of which makes the code wrong, compensate
> >> for each other, making the result right, but the code is
> >> confusing at least:
> >>
> >> #define VSTRING_GET_RESULT(vp, baselen) \
> >> (VSTRING_LEN(vp) > (base_len) ? vstring_end(vp)[-1] : VSTREAM_EOF)
> >>
> >> int vstring_get_flags(VSTRING *vp, VSTREAM *fp, int flags)
> >> {
> >> ssize_t base_len = VSTRING_LEN(vp);
> >> ..
> >> return (VSTRING_GET_RESULT(vp, baselen));
> >> }
> >>
> >> in the macro VSTRING_GET_RESULT(), parameter baselen is not
> >> used, instead, some out-of-context base_len symbol is. While
> >> in all places where VSTRING_GET_RESULT() is used, a non-existing
> >> symbol baselen is passed as an (unused) parameter. At the same
> >> time, local variable base_len exists which meant to be used as
> >> the parameter.
> >
> > That is low-level code that a lot of things depend on. I'll consider
> > this patch if you can demonstrate thet the new source code results
> > in the bit-wise identical object code as the origiinal code.
> >
> > The plan is to have unit tets that cover all Postfxx code, but in
> > until those tests exist, changes to low-level code will require
> > extreme scrutiny.
> >
> > To demonstrate
> >
> > $ cd $original
> > $ make makefiles DEBUG= OPT=
> > $ make
> >
> > $ cd $new
> > $ make makefiles DEBUG= OPT=
> > $ make
> >
> > cmp $original/src/util/vstring_vstream.o $new/src/util/vstring_vstream.o
>
> Actually this is exactly what I did, to be sure. Well, amost:
>
> $ cd src/util
> $ make vstring_vstream.o
> gcc -I. -I../../include -DHAS_DEV_URANDOM -DHAS_PCRE=2 -UUSE_DYNAMIC_LIBS
> -DDEF_SHLIB_DIR=\"no\" \
> -UUSE_DYNAMIC_MAPS -Wmissing-prototypes -Wformat -Wno-comment -fno-common
> -O -I. -DLINUX6 -c vstring_vstream.c
> $ mv vstring_vstream.o vstring_vstream-orig.o
> $ patch -p3 <
> ../../0001-vstring_stream-fix-baselen-vs-base_len-confusion.patch <== this
> is the patch I sent
> $ make vstring_vstream.o
> gcc -I. -I../../include -DHAS_DEV_URANDOM -DHAS_PCRE=2 -UUSE_DYNAMIC_LIBS
> -DDEF_SHLIB_DIR=\"no\" \
> -UUSE_DYNAMIC_MAPS -Wmissing-prototypes -Wformat -Wno-comment -fno-common
> -O -I. -DLINUX6 -c vstring_vstream.c
> $ mv vstring_vstream.o vstring_vstream-fixed.o
> $ ls -l vstring_vstream-*.o
> -rw-r--r-- 1 mjt mjt 3976 Mar 15 18:32 vstring_vstream-fixed.o
> -rw-r--r-- 1 mjt mjt 3976 Mar 15 18:32 vstring_vstream-orig.o
> $ cmp vstring_vstream-*.o
Good man! You could have told me that.
> If you really care about such things, it's best to do this
> locally, without relying on someone else's input.
Of course I'll do it locally, but consider this a time saver for
both of us.
Wietse
_______________________________________________
Postfix-devel mailing list -- [email protected]
To unsubscribe send an email to [email protected]
