'readdir_r' is deprecated. Propose filelister.cpp to use 'readdir'

[ftsf]

When I am building cppcheck using Ubuntu 18.04 or newer, I am seeing this compiler warning.
More reasons for deprecation can be found in manpage of readdir_r
Also available from http://man7.org/linux/man-pages/man3/readdir_r.3.html

$ man 3 readdir_r

Manpage said "It is recommended that applications use readdir(3) instead of
readdir_r(). Furthermore, since version 2.24, glibc deprecates readdir_r()."
More reasoning continue...

Is there any specific reason where readdir_r() must be use by filelister.cpp?

cli/filelister.cpp:197:21: warning: 'readdir_r' is deprecated [-Wdeprecated-declarations]
            while ((readdir_r(dir, &entry, &dir_result) == 0) && (dir_result != nullptr)) {
                    ^
/usr/include/dirent.h:186:28: note: 'readdir_r' has been explicitly marked deprecated here
     __nonnull ((1, 2, 3)) __attribute_deprecated__;
                           ^
/usr/include/x86_64-linux-gnu/sys/cdefs.h:246:51: note: expanded from macro '__attribute_deprecated__'
# define __attribute_deprecated__ __attribute__ ((__deprecated__))

[Daniel Marjamäki]

not everybody is using the latest compilers.

what behaviour do you get if we replace this and then you compile with a 5-10 year old system?

[Haakon Riiser]

Is there a way to handle conflicting recommendations when using multiple --libraries where one should override another? I include both posix.cfg and gnu.cfg, since Linux is my primary platform; gnu.cfg is not a superset of posix.cfg, so I would be missing out on potential warnings if I just include gnu.cfg.

The readdir warning makes sense for posix.cfg, since readdir() is not thread-safe in the current POSIX specification, but when using glibc 2.24 or later, readdir_r() is marked as deprecated in the declaration, and generates compiler warnings even when no warning flags are used.

Here's the reason given by the manual:

It is recommended that applications use readdir(3) instead of
readdir_r(). Furthermore, since version 2.24, glibc deprecates
readdir_r(). The reasons are as follows:

• On systems where NAME_MAX is undefined, calling readdir_r()
  may be unsafe because the interface does not allow the caller
  to specify the length of the buffer used for the returned
  directory entry.

• On some systems, readdir_r() can't read directory entries with
  very long names. When the glibc implementation encounters such
  a name, readdir_r() fails with the error ENAMETOOLONG after
  the final directory entry has been read. On some other systems,
  readdir_r() may return a success status, but the returned d_name
  field may not be null terminated or may be truncated.

• In the current POSIX.1 specification (POSIX.1-2008),
  readdir(3) is not required to be thread-safe. However, in
  modern implementations (including the glibc implementation),
  concurrent calls to readdir(3) that specify different directory
  streams are thread-safe. Therefore, the use of readdir_r()
  is generally unnecessary in multithreaded programs. In cases
  where multiple threads must read from the same directory
  stream, using readdir(3) with external synchronization is still
  preferable to the use of readdir_r(), for the reasons given in
  the points above.

Would be nice to be able to create a new library config, e.g. glibc-v2.24+.cfg that contains:

  <function name="readdir">
    <returnValue type="struct dirent *"/>
    <noreturn>false</noreturn>
    <leak-ignore/>
    <arg nr="1">
      <not-null/>
      <not-uninit/>
      <not-bool/>
    </arg>
  </function>
  <function name="readdir_r">
    <noreturn>false</noreturn>
    <leak-ignore/>
    <returnValue type="int"/>
    <arg nr="1" direction="in">
      <not-null/>
      <not-uninit/>
    </arg>
    <arg nr="2" direction="out">
      <not-null/>
    </arg>
    <arg nr="3" direction="out">
      <not-null/>
    </arg>
    <warn severity="style">This function is deprecated in glibc v2.24 and later; use readdir(3) instead.</warn>
  </function>

I was hoping the readdir entry in this file would override existing entries, but it seems it is only added to the list of checks, no matter the order of the --library flags. Is there a way to make it override, so that my glibc-v2.24+.cfg could be used in combination with posix.cfg?

[Daniel Marjamäki]

we do not have that right now. But it sounds reasonable to me.

[Alexander Mai]

I consider http://pubs.opengroup.org/onlinepubs/9699919799/functions/readdir.html to be the ultimate reference, not man7.org...

[Nicholas Nezis]

Is anyone working this? This is a blocker for us using cppcheck on Ubuntu 18.04. Does cppcheck need to keep supporting 5-10 year old OS? I would think those using the older OS would be ok using an older version of cppcheck. But those wanting to keep moving forward need a version of cppcheck that is no longer using a deprecated api call. I'm willing to help make the code edits. Thanks.

[Daniel Marjamäki]

I do not think that anybody is working on this.

I think we should try our best to support 5-10 year old systems. We still do not use all C++11 features in our code for backwards compatibility reasons.

I'm willing to help make the code edits.

Great. I'd assume you just need to tweak the code in cppcheck/lib/library.cpp ... if a <function name="foo">.. is parsed then I'd say it would be fine that all the old configuration for foo is removed.

Ideally we should also allow overloaded functions.. but that would be a much bigger and more difficult change. If that is something you want to look at feel free to do it.