clisp-devel

multiple unsequenced modifications

[Sam S. <sd...@gn...>]

Hi,

I get this:
--8<---------------cut here---------------start------------->8---
modules/rawsock/rawsock.c:893:48: warning: multiple unsequenced
  modifications to 'STACK' [-Wunsequenced]
--8<---------------cut here---------------end--------------->8---

because of

--8<---------------cut here---------------start------------->8---
  struct addrinfo hints = {addrinfo_flags(),
                           check_socket_domain(popSTACK()),
                           check_socket_type(popSTACK()),
                           get_socket_protocol(popSTACK()),
                           0,NULL,NULL,NULL};
--8<---------------cut here---------------end--------------->8---

It appears that the code _should_ DTRT in all commercial compilers and
GCC 3.4.6 or later.

https://stackoverflow.com/q/19881803/850781
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=11633
http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#430

What should be done about this?

It is easy to rewrite the code (replace popSTACK() with STACK_??? and
add skipSTACK() at the end) but this would be a lie because
addrinfo_flags() modifies STACK too, so we are relying on the sequential
evaluation of all slots in the initialization no matter what.

The only honest way to avoid the issue is

--8<---------------cut here---------------start------------->8---
  struct addrinfo hints = {0,0,0,0,0,NULL,NULL,NULL};
  hints.ai_flags = addrinfo_flags();
  hints.ai_family = check_socket_domain(popSTACK());
  hints.ai_socktype = check_socket_type(popSTACK());
  hints.ai_protocol = get_socket_protocol(popSTACK());
--8<---------------cut here---------------end--------------->8---

Bruno, does it make sense?
It is worth bothering with?

Thanks

-- 
Sam Steingold (http://sds.podval.org/) on darwin Ns 10.3.1504
http://steingoldpsychology.com http://www.childpsy.net http://camera.org
http://americancensorship.org https://jihadwatch.org http://iris.org.il
"Bravery is being the only one who knows you're afraid." --Franklin P. Jones

Re: multiple unsequenced modifications

[Bruno H. <br...@cl...>]

Hi Sam,

> I get this:
> --8<---------------cut here---------------start------------->8---
> modules/rawsock/rawsock.c:893:48: warning: multiple unsequenced
>   modifications to 'STACK' [-Wunsequenced]
> --8<---------------cut here---------------end--------------->8---
> 
> because of
> 
> --8<---------------cut here---------------start------------->8---
>   struct addrinfo hints = {addrinfo_flags(),
>                            check_socket_domain(popSTACK()),
>                            check_socket_type(popSTACK()),
>                            get_socket_protocol(popSTACK()),
>                            0,NULL,NULL,NULL};
> --8<---------------cut here---------------end--------------->8---

GCC told you about one problem with this code.

The other problem is that the code assumes a certain order of
the members of 'struct addrinfo'. Which is not guaranteed by POSIX: [1] says
   "The <netdb.h> header shall define the addrinfo structure, which
    shall include at least the following members:"

> The only honest way to avoid the issue is
> 
> --8<---------------cut here---------------start------------->8---
>   struct addrinfo hints = {0,0,0,0,0,NULL,NULL,NULL};
>   hints.ai_flags = addrinfo_flags();
>   hints.ai_family = check_socket_domain(popSTACK());
>   hints.ai_socktype = check_socket_type(popSTACK());
>   hints.ai_protocol = get_socket_protocol(popSTACK());
> --8<---------------cut here---------------end--------------->8---

This is better, but it too suffers from the second problem.

How about this?

  struct addrinfo hints;
  memset(&hints,0,sizeof(hints));
  hints.ai_flags = addrinfo_flags();
  hints.ai_family = check_socket_domain(popSTACK());
  hints.ai_socktype = check_socket_type(popSTACK());
  hints.ai_protocol = get_socket_protocol(popSTACK());

> It is worth bothering with?

Definitely!

Bruno

[1] http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/netdb.h.html

Re: multiple unsequenced modifications

[Sam S. <sd...@gn...>]

Hi Bruno,

> * Bruno Haible <oe...@py...t> [2017-12-07 12:00:16 +0100]:
>
>   memset(&hints,0,sizeof(hints));

Do we need to wrap this in begin_system_call()/end_system_call()?
I see a certain level of inconsistency about this. ;-(

Thanks!

-- 
Sam Steingold (http://sds.podval.org/) on darwin Ns 10.3.1504
http://steingoldpsychology.com http://www.childpsy.net http://no2bds.org
https://jihadwatch.org http://iris.org.il http://mideasttruth.com
To understand recursion, one has to understand recursion first.

Re: multiple unsequenced modifications

[Sam S. <sd...@gn...>]

> * Bruno Haible <oe...@py...t> [2017-12-07 12:00:16 +0100]:
>
>> modules/rawsock/rawsock.c:893:48: warning: multiple unsequenced
>>   modifications to 'STACK' [-Wunsequenced]
>> It is worth bothering with?
>
> Definitely!

How is this different from -Wsequence-point ?

(cf https://sourceforge.net/p/clisp/patches/54/)

Thanks!

-- 
Sam Steingold (http://sds.podval.org/) on darwin Ns 10.3.1504
http://steingoldpsychology.com http://www.childpsy.net https://ffii.org
http://think-israel.org http://mideasttruth.com https://jihadwatch.org
To avoid fatigue, one must sleep 8 hours every day. And 8 hours every night too.

Re: multiple unsequenced modifications

[Bruno H. <br...@cl...>]

Sam asked on 2017-12-07:
> >> modules/rawsock/rawsock.c:893:48: warning: multiple unsequenced
> >>   modifications to 'STACK' [-Wunsequenced]
> >> It is worth bothering with?
> >
> > Definitely!
> 
> How is this different from -Wsequence-point ?

Look up the manuals: GCC [1] has -Wsequence-point, whereas clang [2]
has -Wsequence-point and -Wunsequenced, and they are synonymous [3].

Bruno

[1] https://gcc.gnu.org/onlinedocs/gcc-7.2.0/gcc/Warning-Options.html
[2] https://clang.llvm.org/docs/DiagnosticsReference.html
[3] https://clang.llvm.org/docs/DiagnosticsReference.html#wsequence-point