libsidplayfp / sidplayfp

Console SID/MUS player
GNU General Public License v2.0
34 stars 8 forks source link

PATH_MAX undefined #24

Closed Rhialto closed 8 months ago

Rhialto commented 8 months ago

I tried compiling version 2.5.0 on NetBSD and I got this error:

src/IniConfig.cpp: In member function 'void IniConfig::readSidplay2(iniHandler& ':
src/IniConfig.cpp:325:21: error: 'PATH_MAX' was not declared in this scope
  325 |         char buffer[PATH_MAX];
      |                     ^~~~~~~~
src/IniConfig.cpp:326:18: error: 'buffer' was not declared in this scope; did you mean 'setbuffer'?
  326 |         snprintf(buffer, PATH_MAX, "%sSonglengths.txt", PKGDATADIR);
      |                  ^~~~~~
      |                  setbuffer
gmake: *** [Makefile:792: src/IniConfig.o] Error 1

The relevant source code seems to look like

#if !defined _WIN32 && defined HAVE_UNISTD_H
    if (sidplay2_s.database.empty())
    {   
        char buffer[PATH_MAX];
        snprintf(buffer, PATH_MAX, "%sSonglengths.txt", PKGDATADIR);
        if (::access(buffer, R_OK) == 0)
            sidplay2_s.database.assign(buffer);
    }
#endif

However, PATH_MAX isn't defined in <unistd.h> but in <limits.h>. See POSIX: https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/limits.h.html and https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/unistd.h.html .

And it's not even guaranteed to be there anyway, if I interpret this wording correctly: "A definition of one of the symbolic constants in the following list shall be omitted from the header on specific implementations where the corresponding value is equal to or greater than the stated minimum, but where the value can vary depending on the file to which it is applied. The actual value supported for a specific pathname shall be provided by the pathconf() function."

By the way, what sound output possibilities are there? Only alsa and pulse-audio?

drfiemost commented 8 months ago

Thanks for the report, I've moved to std::string so there's no need for PATH_MAX anymore.

As for the audio output there is also oss and a few more options if you have libout123 from the mpg123 package installed.

Rhialto commented 8 months ago

Perfect!

As for the audio output, I'll investigate what is needed for the configure script to see those options. Normally, in pkgsrc, packages that are not declared as requirements are hidden from configure, to prevent surprises. I expect I will have to add some, and then it will work. Thanks!