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 <m...@tls.msk.ru>
> ---
> 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 -- postfix-devel@postfix.org
> To unsubscribe send an email to postfix-devel-le...@postfix.org
> 
_______________________________________________
Postfix-devel mailing list -- postfix-devel@postfix.org
To unsubscribe send an email to postfix-devel-le...@postfix.org

Reply via email to