diff mbox series

[4/7] linux: Fix setsockopt fallback

Message ID 20210705183027.3804093-5-adhemerval.zanella@linaro.org
State Committed
Headers
Series Linux 64-bit time_t socket fixes |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent

Commit Message

Adhemerval Zanella Netto July 5, 2021, 6:30 p.m. UTC 
  The final 2 arguments for SO_TIMESTAMP/SO_TIMESTAMPNS are being set
wrongly.

Checked on x86_64-linux-gnu and i686-linux-gnu.
---
 sysdeps/unix/sysv/linux/setsockopt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Florian Weimer July 5, 2021, 7:07 p.m. UTC | #1 
* Adhemerval Zanella via Libc-alpha:

> The final 2 arguments for SO_TIMESTAMP/SO_TIMESTAMPNS are being set
> wrongly.
>
> Checked on x86_64-linux-gnu and i686-linux-gnu.
> ---
>  sysdeps/unix/sysv/linux/setsockopt.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/sysdeps/unix/sysv/linux/setsockopt.c b/sysdeps/unix/sysv/linux/setsockopt.c
> index a4780a9d33..fae7305c9d 100644
> --- a/sysdeps/unix/sysv/linux/setsockopt.c
> +++ b/sysdeps/unix/sysv/linux/setsockopt.c
> @@ -78,7 +78,7 @@ setsockopt32 (int fd, int level, int optname, const void *optval,
>  	  optname = COMPAT_SO_TIMESTAMP_OLD;
>  	if (optname == COMPAT_SO_TIMESTAMPNS_NEW)
>  	  optname = COMPAT_SO_TIMESTAMPNS_OLD;
> -	r = setsockopt_syscall (fd, level, optname, NULL, 0);
> +	r = setsockopt_syscall (fd, level, optname, optval, len);
>        }
>        break;
>      }

Maybe add a comment that the recvmsg code does conversation, so the old
socket option is also acceptable?

The patch itself looks okay to me.

Thanks,
Florian
Adhemerval Zanella Netto July 6, 2021, 1:07 p.m. UTC | #2 
On 05/07/2021 16:07, Florian Weimer wrote:
> * Adhemerval Zanella via Libc-alpha:
> 
>> The final 2 arguments for SO_TIMESTAMP/SO_TIMESTAMPNS are being set
>> wrongly.
>>
>> Checked on x86_64-linux-gnu and i686-linux-gnu.
>> ---
>>  sysdeps/unix/sysv/linux/setsockopt.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/sysdeps/unix/sysv/linux/setsockopt.c b/sysdeps/unix/sysv/linux/setsockopt.c
>> index a4780a9d33..fae7305c9d 100644
>> --- a/sysdeps/unix/sysv/linux/setsockopt.c
>> +++ b/sysdeps/unix/sysv/linux/setsockopt.c
>> @@ -78,7 +78,7 @@ setsockopt32 (int fd, int level, int optname, const void *optval,
>>  	  optname = COMPAT_SO_TIMESTAMP_OLD;
>>  	if (optname == COMPAT_SO_TIMESTAMPNS_NEW)
>>  	  optname = COMPAT_SO_TIMESTAMPNS_OLD;
>> -	r = setsockopt_syscall (fd, level, optname, NULL, 0);
>> +	r = setsockopt_syscall (fd, level, optname, optval, len);
>>        }
>>        break;
>>      }
> 
> Maybe add a comment that the recvmsg code does conversation, so the old
> socket option is also acceptable?

I added the following comment:

  /* The expected type for the option is an 'int' for both
     types of timestamp formats, so there is no need to convert it.  */

> 
> The patch itself looks okay to me.
Florian Weimer July 6, 2021, 2:08 p.m. UTC | #3 
* Adhemerval Zanella:

> On 05/07/2021 16:07, Florian Weimer wrote:
>> * Adhemerval Zanella via Libc-alpha:
>> 
>>> The final 2 arguments for SO_TIMESTAMP/SO_TIMESTAMPNS are being set
>>> wrongly.
>>>
>>> Checked on x86_64-linux-gnu and i686-linux-gnu.
>>> ---
>>>  sysdeps/unix/sysv/linux/setsockopt.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/sysdeps/unix/sysv/linux/setsockopt.c b/sysdeps/unix/sysv/linux/setsockopt.c
>>> index a4780a9d33..fae7305c9d 100644
>>> --- a/sysdeps/unix/sysv/linux/setsockopt.c
>>> +++ b/sysdeps/unix/sysv/linux/setsockopt.c
>>> @@ -78,7 +78,7 @@ setsockopt32 (int fd, int level, int optname, const void *optval,
>>>  	  optname = COMPAT_SO_TIMESTAMP_OLD;
>>>  	if (optname == COMPAT_SO_TIMESTAMPNS_NEW)
>>>  	  optname = COMPAT_SO_TIMESTAMPNS_OLD;
>>> -	r = setsockopt_syscall (fd, level, optname, NULL, 0);
>>> +	r = setsockopt_syscall (fd, level, optname, optval, len);
>>>        }
>>>        break;
>>>      }
>> 
>> Maybe add a comment that the recvmsg code does conversation, so the old
>> socket option is also acceptable?
>
> I added the following comment:
>
>   /* The expected type for the option is an 'int' for both
>      types of timestamp formats, so there is no need to convert it.  */

And it's possible to request the old timestamp because of the
conversation in recvmsg/recvmmsg from the old to the new timestamp.

Thanks,
Florian
diff mbox series

Patch

diff --git a/sysdeps/unix/sysv/linux/setsockopt.c b/sysdeps/unix/sysv/linux/setsockopt.c
index a4780a9d33..fae7305c9d 100644
--- a/sysdeps/unix/sysv/linux/setsockopt.c
+++ b/sysdeps/unix/sysv/linux/setsockopt.c
@@ -78,7 +78,7 @@  setsockopt32 (int fd, int level, int optname, const void *optval,
 	  optname = COMPAT_SO_TIMESTAMP_OLD;
 	if (optname == COMPAT_SO_TIMESTAMPNS_NEW)
 	  optname = COMPAT_SO_TIMESTAMPNS_OLD;
-	r = setsockopt_syscall (fd, level, optname, NULL, 0);
+	r = setsockopt_syscall (fd, level, optname, optval, len);
       }
       break;
     }