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
This is, for example, how I verified the migration from "Postfix
bool" to a different definition for c23 compliance.
Wietse
> Fix this by renaming one of the two names.
>
> Signed-off-by: Michael Tokarev <[email protected]>
> ---
> v2: fix the commit message to include macro #define
> (no changes in the code)
>
> src/util/vstring_vstream.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/src/util/vstring_vstream.c b/src/util/vstring_vstream.c
> index 451cc5017..3bf5e050f 100644
> --- a/src/util/vstring_vstream.c
> +++ b/src/util/vstring_vstream.c
> @@ -123,7 +123,7 @@
> /*
> * Macro to return the last character added to a VSTRING, for consistency.
> */
> -#define VSTRING_GET_RESULT(vp, baselen) \
> +#define VSTRING_GET_RESULT(vp, base_len) \
> (VSTRING_LEN(vp) > (base_len) ? vstring_end(vp)[-1] : VSTREAM_EOF)
>
> /* vstring_get_flags - read line from file, keep newline */
> @@ -142,7 +142,7 @@ int vstring_get_flags(VSTRING *vp, VSTREAM *fp, int
> flags)
> break;
> }
> VSTRING_TERMINATE(vp);
> - return (VSTRING_GET_RESULT(vp, baselen));
> + return (VSTRING_GET_RESULT(vp, base_len));
> }
>
> /* vstring_get_flags_nonl - read line from file, strip newline */
> @@ -158,7 +158,7 @@ int vstring_get_flags_nonl(VSTRING *vp, VSTREAM *fp,
> int flags)
> while ((c = VSTREAM_GETC(fp)) != VSTREAM_EOF && c != '\n')
> VSTRING_ADDCH(vp, c);
> VSTRING_TERMINATE(vp);
> - return (c == '\n' ? c : VSTRING_GET_RESULT(vp, baselen));
> + return (c == '\n' ? c : VSTRING_GET_RESULT(vp, base_len));
> }
>
> /* vstring_get_flags_null - read null-terminated string from file */
> @@ -174,7 +174,7 @@ int vstring_get_flags_null(VSTRING *vp, VSTREAM *fp,
> int flags)
> while ((c = VSTREAM_GETC(fp)) != VSTREAM_EOF && c != 0)
> VSTRING_ADDCH(vp, c);
> VSTRING_TERMINATE(vp);
> - return (c == 0 ? c : VSTRING_GET_RESULT(vp, baselen));
> + return (c == 0 ? c : VSTRING_GET_RESULT(vp, base_len));
> }
>
> /* vstring_get_flags_bound - read line from file, keep newline, up to bound
> */
> @@ -197,7 +197,7 @@ int vstring_get_flags_bound(VSTRING *vp, VSTREAM *fp,
> int flags,
> break;
> }
> VSTRING_TERMINATE(vp);
> - return (VSTRING_GET_RESULT(vp, baselen));
> + return (VSTRING_GET_RESULT(vp, base_len));
> }
>
> /* vstring_get_flags_nonl_bound - read line from file, strip newline, up to
> bound */
> @@ -217,7 +217,7 @@ int vstring_get_flags_nonl_bound(VSTRING *vp, VSTREAM
> *fp, int flags,
> while (bound-- > 0 && (c = VSTREAM_GETC(fp)) != VSTREAM_EOF && c != '\n')
> VSTRING_ADDCH(vp, c);
> VSTRING_TERMINATE(vp);
> - return (c == '\n' ? c : VSTRING_GET_RESULT(vp, baselen));
> + return (c == '\n' ? c : VSTRING_GET_RESULT(vp, base_len));
> }
>
> /* vstring_get_flags_null_bound - read null-terminated string from file */
> @@ -237,7 +237,7 @@ int vstring_get_flags_null_bound(VSTRING *vp, VSTREAM
> *fp, int flags,
> while (bound-- > 0 && (c = VSTREAM_GETC(fp)) != VSTREAM_EOF && c != 0)
> VSTRING_ADDCH(vp, c);
> VSTRING_TERMINATE(vp);
> - return (c == 0 ? c : VSTRING_GET_RESULT(vp, baselen));
> + return (c == 0 ? c : VSTRING_GET_RESULT(vp, base_len));
> }
>
> #ifdef TEST
> --
> 2.39.5
>
> _______________________________________________
> Postfix-devel mailing list -- [email protected]
> To unsubscribe send an email to [email protected]
>
_______________________________________________
Postfix-devel mailing list -- [email protected]
To unsubscribe send an email to [email protected]