Details

    • Type: Bug
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 0.9.9
    • Fix Version/s: 0.9.12-incubating
    • Component/s: RDP
    • Labels:
      None

      Description

      A function is now deprecated in /src/protocols/rdp/rdp_fs.c:

      http://man7.org/linux/man-pages/man3/readdir_r.3.html

      I get this during builds (note, I've got a Nix expression building this in a chroot'd sandbox each time):

      ...
        CC       guac_rdpdr/guacdr_la-rdpdr_fs_service.lo
        CC       guac_rdpdr/guacdr_la-rdpdr_messages.lo
        CC       guac_rdpdr/guacdr_la-rdpdr_printer.lo
        CC       guac_rdpdr/guacdr_la-rdpdr_service.lo
        CC       guacdr_la-rdp_fs.lo
      rdp_fs.c: In function 'guac_rdp_fs_read_dir':
      rdp_fs.c:563:5: error: 'readdir_r' is deprecated [-Werror=deprecated-declarations]
           if (readdir_r(file->dir, &(file->__dirent), &result))
           ^
      In file included from rdp_fs.h:44:0,
                       from rdp_fs.c:25:
      /nix/store/8ic0jwg3p5vcwx52k4781n987hmv0bks-glibc-2.24-dev/include/dirent.h:183:12: note: declared here
       extern int readdir_r (DIR *__restrict __dirp,
                  ^
      cc1: all warnings being treated as errors
      make[3]: *** [Makefile:1197: guacdr_la-rdp_fs.lo] Error 1
      make[3]: Leaving directory '/tmp/nix-build-guacamole-0.9.9.drv-0/incubator-guacamole-server-6c0862e82da8ee737ee655c7815b1851de6d0488-src/src/protocols/rdp'
      make[2]: *** [Makefile:736: all] Error 2
      make[2]: Leaving directory '/tmp/nix-build-guacamole-0.9.9.drv-0/incubator-guacamole-server-6c0862e82da8ee737ee655c7815b1851de6d0488-src/src/protocols/rdp'
      make[1]: *** [Makefile:474: all-recursive] Error 1
      make[1]: Leaving directory '/tmp/nix-build-guacamole-0.9.9.drv-0/incubator-guacamole-server-6c0862e82da8ee737ee655c7815b1851de6d0488-src'
      make: *** [Makefile:406: all] Error 2
      

        Activity

        Hide
        mike.jumper Michael Jumper added a comment -

        Well, clearly we need to address this somehow.

        That said, I cannot fathom why glibc is taking it upon themselves to deprecate a standard function. The documented justification:

        It is expected that a future version of POSIX.1 will make readdir_r() obsolete, and require that readdir() be thread-safe when concurrently employed on different directory streams.

        seems twisted, since the key point is that the current standard does no such thing. It strikes me as odd that a libc implementation would intentionally break their previous compatibility with the actual existing standard, in the hope that their vision for a different standard will become reality and provide absolution.

        Anyway, on to what this would mean for the guacamole-server source:

        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.

        Using readdir() instead of readdir_r() will require adding additional synchronization to prevent client threads trampling each other during concurrent directory reads.

        Show
        mike.jumper Michael Jumper added a comment - Well, clearly we need to address this somehow. That said, I cannot fathom why glibc is taking it upon themselves to deprecate a standard function. The documented justification: It is expected that a future version of POSIX.1 will make readdir_r() obsolete, and require that readdir() be thread-safe when concurrently employed on different directory streams. seems twisted, since the key point is that the current standard does no such thing. It strikes me as odd that a libc implementation would intentionally break their previous compatibility with the actual existing standard, in the hope that their vision for a different standard will become reality and provide absolution. Anyway, on to what this would mean for the guacamole-server source: 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. Using readdir() instead of readdir_r() will require adding additional synchronization to prevent client threads trampling each other during concurrent directory reads.
        Hide
        mike.jumper Michael Jumper added a comment -

        Using readdir() instead of readdir_r() will require adding additional synchronization to prevent client threads trampling each other during concurrent directory reads.

        Actually ... there shouldn't be any concurrent directory reads. It may well be safe to simply use readdir(). While there are different threads which will access the same underlying filesystem, they won't access the same file handles and thus will not access the same directory streams.

        Show
        mike.jumper Michael Jumper added a comment - Using readdir() instead of readdir_r() will require adding additional synchronization to prevent client threads trampling each other during concurrent directory reads. Actually ... there shouldn't be any concurrent directory reads. It may well be safe to simply use readdir() . While there are different threads which will access the same underlying filesystem, they won't access the same file handles and thus will not access the same directory streams.

          People

          • Assignee:
            mike.jumper Michael Jumper
            Reporter:
            tomberek+guacamole@gmail.com tomberek
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development