"mbsspn" and "mbschr" missing in some files in /lib/

"mbsspn" and "mbschr" missing in some files in /lib/

[Bjarni Ingi Gislason]

  While experimenting using the module "posixcheck" included in
"groff" I got these warnings:

N.B.  Some lines are folded.

  CC       lib/libgnu_a-chdir-long.o
../lib/chdir-long.c: In function 'chdir_long':
../lib/chdir-long.c:129:23: warning: call to 'strspn' declared
with attribute warning: strspn cannot work correctly on character
strings in multibyte locales - use mbsspn if you care about
internationalization [-Wattribute-warning]
  129 |     n_leading_slash = strspn (dir, "/");
      |                       ^~~~~~~~~~~~~~~~~
In function 'find_non_slash',
    inlined from 'chdir_long' at ../lib/chdir-long.c:152:15:
../lib/chdir-long.c:89:20: warning: call to 'strspn' declared
with attribute warning: strspn cannot work correctly on character
strings in multibyte locales - use mbsspn if you care about
internationalization [-Wattribute-warning]
   89 |   size_t n_slash = strspn (s, "/");
      |                    ^~~~~~~~~~~~~~~
In function 'find_non_slash',
    inlined from 'chdir_long' at ../lib/chdir-long.c:184:15:
../lib/chdir-long.c:89:20: warning: call to 'strspn' declared
with attribute warning: strspn cannot work correctly on character
strings in multibyte locales - use mbsspn if you care about
internationalization [-Wattribute-warning]
   89 |   size_t n_slash = strspn (s, "/");
      |                    ^~~~~~~~~~~~~~~

-.-.

  CC       lib/libgnu_a-mbschr.o
../lib/mbschr.c: In function 'mbschr':
 ../lib/mbschr.c:68:12: warning: call to 'strchr' declared with
attribute warning: strchr cannot work correctly on character
strings in some multibyte locales - use mbschr if you care about
internationalization [-Wattribute-warning]
   68 |     return strchr (string, c);
      |            ^~~~~~~~~~~~~~~~~~
  CC       lib/libgnu_a-mbsspn.o
../lib/mbsspn.c: In function 'mbsspn':
 ../lib/mbsspn.c:135:12: warning: call to 'strspn' declared with
attribute warning: strspn cannot work correctly on character
strings in multibyte locales - use mbsspn if you care about
internationalization [-Wattribute-warning]
  135 |     return strspn (string, reject);
      |            ^~~~~~~~~~~~~~~~~~~~~~~

-.-

  CC       lib/libgnu_a-tempname.o
../lib/tempname.c: In function 'try_tempname_len':
../lib/tempname.c:230:10: warning: call to 'strspn' declared
with attribute warning: strspn cannot work correctly on character
strings in multibyte locales - use mbsspn if you care about
internationalization [-Wattribute-warning]
  230 |       || strspn (&tmpl[len - x_suffix_len - suffixlen],
"X") < x_suffix_len)
      |         
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

-.-.

  It looks like "mbsspn" ("mbschr") needs to be selected in
these files (and others?) when the corresponding modules are
listed in "bootstrap.conf".

Re: "mbsspn" and "mbschr" missing in some files in /lib/

[Bruno Haible]

Bjarni Ingi Gislason wrote:
>   CC       lib/libgnu_a-chdir-long.o
> ../lib/chdir-long.c: In function 'chdir_long':
> ../lib/chdir-long.c:129:23: warning: call to 'strspn' declared
> with attribute warning: strspn cannot work correctly on character
> strings in multibyte locales - use mbsspn if you care about
> internationalization [-Wattribute-warning]
>   129 |     n_leading_slash = strspn (dir, "/");
>       |                       ^~~~~~~~~~~~~~~~~

>   It looks like "mbsspn" ("mbschr") needs to be selected in
> these files (and others?) when the corresponding modules are
> listed in "bootstrap.conf".

If you want to use the function 'mbsspn', then yes you need to import
the corresponding module ('mbsspn') via gnulib-tool.

https://www.gnu.org/software/gnulib/manual/html_node/Finding-POSIX-substitutes.html
step 4.

Bruno

Re: "mbsspn" and "mbschr" missing in some files in /lib/

[Paul Eggert]

On 2/1/24 14:55, Bruno Haible wrote:
> If you want to use the function 'mbsspn', then yes you need to import
> the corresponding module ('mbsspn') via gnulib-tool.

Sure, but in this case Bjarni didn't want to use that module; it was 
pulled in by a dependency. And as far as I can tell, the diagnostics he 
got were (in some sense) all false positives.

How about if we separate out the "if you care about 
internationalization" diagnostics into a separate category, enabled if 
GNULIB_UNUSUAL_ENCODING_CHECK is defined, instead of GNULIB_POSIXCHECK? 
These diagnostics aren't so much about POSIX as they are about GB18030 
and other unusual encodings, and since they generate so many false 
positives they're good candidates for a separate category.

Re: i18n warnings

[Bruno Haible]

Paul Eggert wrote:
> And as far as I can tell, the diagnostics he 
> got were (in some sense) all false positives.

Generally, if the second argument to strchr is a variable, it can contain
any character, thus the warning is warranted. Whereas if the second argument
to strchr is a literal character, such as ' ' or '\n', there is no risk,
and the warning is a false alarm.

> How about if we separate out the "if you care about 
> internationalization" diagnostics into a separate category, enabled if 
> GNULIB_UNUSUAL_ENCODING_CHECK is defined, instead of GNULIB_POSIXCHECK? 
> These diagnostics aren't so much about POSIX as they are about GB18030 
> and other unusual encodings, and since they generate so many false 
> positives they're good candidates for a separate category.

While GNULIB_POSIXCHECK produces warnings for portability and warnings
for i18n support, I don't think it would be good to separate their
activation. The most probable consequence would be that people deactivate
the i18n warnings, and thus get i18n bugs at runtime on glibc systems.

It would be better to think about how to reduce the false alarms.

For example by analyzing the second argument at compile-time:

* For functions whose second argument is a string, in C++, it is possible
  to analyze a string at compile time, using the techniques that implement
  <format>. (I'm not saying it's easy. But it's possible.)

  Maybe we should implement this, and give the advice to set
  CC="gcc -x c++" or CC="g++" together with GNULIB_POSIXCHECK?

* For functions whose second argument is a character, such as 'strchr',
  we can probably play with __builtin_constant_p.

Bruno

Re: i18n warnings

[Paul Eggert]

On 2/2/24 01:53, Bruno Haible wrote:
> * For functions whose second argument is a character, such as 'strchr',
>    we can probably play with __builtin_constant_p.

Another idea is to suppress the warnings in a library module where the 
warnings are a false alarm. For example, mbschr.c calls strchr only when 
MB_CUR_MAX is 1, so that warning about strchr is bogus. As I vaguely 
recall, the warnings Bjarni mentioned are in this category.