Re: Fix trim2 uninitialized GCC warning

[Bruno Haible]

Hello Georg,

> However, to eliminate the warning, the code also can be simplified like this:
> 
> ```
> diff --git a/lib/trim.c b/lib/trim.c
> index 162c43e1de..bfd743cf0f 100644
> --- a/lib/trim.c
> +++ b/lib/trim.c
> @@ -30,12 +30,6 @@
>  #include "mbiter.h"
>  #include "xalloc.h"
>  
> -/* Use this to suppress gcc's "...may be used before initialized" warnings. 
> */
> -#if defined GCC_LINT || defined lint
> -# define IF_LINT(Code) Code
> -#else
> -# define IF_LINT(Code) /* empty */
> -#endif
>  
>  char *
>  trim2 (const char *s, int how)
> @@ -66,7 +60,6 @@ trim2 (const char *s, int how)
>        if (how != TRIM_LEADING)
>          {
>            unsigned int state = 0;
> -          char *r IF_LINT (= NULL); /* used only while state = 2 */
>  
>            mbi_init (i, d, strlen (d));
>  
> @@ -87,7 +80,7 @@ trim2 (const char *s, int how)
>                if (state == 1 && mb_isspace (mbi_cur (i)))
>                  {
>                    state = 2;
> -                  r = (char *) mbi_cur_ptr (i);
> +                  *(char *) mbi_cur_ptr (i) = 0;
>                  }
>                else if (state == 2 && mb_isspace (mbi_cur (i)))
>                  {
> @@ -98,9 +91,6 @@ trim2 (const char *s, int how)
>                    state = 1;
>                  }
>              }
> -
> -          if (state == 2)
> -            *r = '\0';
>          }
>      }
>    else
> ```
> 
> In that way the warning is reliably eliminated, there is no need anymore
> for clever macro tricks and arguably the code is now easier to grasp.

Thank you for the constructive proposal.

Before modifying any algorithmic code, I want to have a unit test — because
otherwise the risk of regression too high.

So, I added unit tests that
  A. test a variety of ASCII strings with ASCII spaces in the C locale,
  B. test the same thing in multibyte locales,
  C. test additionally some Unicode spaces in these multibyte locales.

What I saw is that the tests B fail:

  PASS: test-trim1.sh
  FAIL: test-trim2.sh
  FAIL: test-trim3.sh

  FAIL: test-trim2.sh
  ===================

  test-trim.c:54: assertion 'strcmp (result, "") == 0' failed
  Aborted
  FAIL test-trim2.sh (exit status: 134)

  FAIL: test-trim3.sh
  ===================

  test-trim.c:54: assertion 'strcmp (result, "") == 0' failed
  Aborted
  FAIL test-trim3.sh (exit status: 134)

In other words, the function is already buggy since the beginning, since
it produces different results for ASCII inputs depending on whether the
locale is multibyte or unibyte!

After applying your patch, the same tests continued to fail.

So, I went for a more radical simplification. It does not elicit a warning
any more.

Thanks for your input; without it, we would still be sitting on this bug
in the future...


2023-04-02  Bruno Haible  <bruno@clisp.org>

        trim: Fix trim_trailing result in multibyte locales.
        * lib/trim.c (trim2): Simplify algorithm for trim_trailing in multibyte
        locales, to use 2 instead of 3 states.

        trim: Add tests.
        * tests/test-trim.c: New file.
        * tests/test-trim1.sh: New file.
        * tests/test-trim2.sh: New file.
        * tests/test-trim3.sh: New file.
        * modules/trim-tests: New file.

0001-trim-Add-tests.patch
Description: Text Data

0002-trim-Fix-trim_trailing-result-in-multibyte-locales.patch
Description: Text Data