ppp-project / ppp

Paul's PPP Package: PPP daemon and associated utilities | Official GitHub repo: https://github.com/ppp-project/ppp
https://github.com/ppp-project/ppp
Other
389 stars 231 forks source link

Fix warning messages related to lcp_rtt_* functions #457

Closed enaess closed 6 months ago

enaess commented 1 year ago

Compiling the latest mainline on Solaris adds additional warning messages, we should address these. And why use the keyword "volatile" in these circumstances?

  lcp.c: In function ‘lcp_rtt_update_buffer’:
  lcp.c:2310:15: warning: passing argument 1 of ‘msync’ discards ‘volatile’ qualifier from pointer target type [-Wdiscarded-qualifiers]
       if (msync(lcp_rtt_buffer, LCP_RTT_FILE_SIZE, MS_ASYNC) < 0)
                 ^~~~~~~~~~~~~~
  In file included from lcp.c:54:0:
  /usr/include/sys/mman.h:204:12: note: expected ‘void *’ but argument is of type ‘volatile u_int32_t * {aka volatile unsigned int *}’
   extern int msync(void *, size_t, int);
              ^~~~~
  lcp.c: In function ‘lcp_rtt_open_file’:
  lcp.c:2444:9: warning: passing argument 1 of ‘memset’ discards ‘volatile’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    memset(lcp_rtt_buffer, 0, LCP_RTT_FILE_SIZE);
           ^~~~~~~~~~~~~~
  In file included from /usr/include/string.h:12:0,
                   from lcp.c:48:
  /usr/include/iso/string_iso.h:85:14: note: expected ‘void *’ but argument is of type ‘volatile u_int32_t * {aka volatile unsigned int *}’
   extern void *memset(void *, int, size_t);
                ^~~~~~
  lcp.c: In function ‘lcp_rtt_close_file’:
  lcp.c:2460:16: warning: passing argument 1 of ‘munmap’ discards ‘volatile’ qualifier from pointer target type [-Wdiscarded-qualifiers]
       if (munmap(lcp_rtt_buffer, LCP_RTT_FILE_SIZE) < 0)
                  ^~~~~~~~~~~~~~
  In file included from lcp.c:54:0:
  /usr/include/sys/mman.h:202:12: note: expected ‘void *’ but argument is of type ‘volatile u_int32_t * {aka volatile unsigned int *}’
   extern int munmap(void *, size_t);
              ^~~~~~
enaess commented 1 year ago

@rfc1036

rfc1036 commented 1 year ago

I think that the purpose is to prevent reordering by the compiler.

paulusmack commented 1 year ago

The buffer is volatile because other programs could potentially mmap it and access it concurrently, and so we want to limit reordering of accesses by the compiler. Volatile is not a complete fix, though, because the CPU can still reorder accesses, and to fix it properly we would need barrier instructions, which tends to get machine-dependent.

It is possible to make the accesses volatile,rather than the data, i.e. use casts to volatile where the buffer entries are read or written rather than making the data structure itself volatile.

Neustradamus commented 1 year ago

About Solaris:

cc: @carlsonj, @RICCIARDI-Adrien.

Neustradamus commented 10 months ago

@enaess: This bug always exists?

paulusmack commented 6 months ago

Fixed by fafbfdf