Re: [LTP] [RFC PATCH 1/1] regen.sh: Use intptr_t for tst_syscall return

[Teo Couprie Diaz <teo.coupriediaz@arm.com>]

Hello,

On 31/10/2022 13:32, Cyril Hrubis wrote:
> Hi!
>> Some syscalls directly return pointers, like brk or mmap. int is currently
>> used for the return value in tst_syscall but is not large enough
>> to guarantee that such a returned value will fit.
>> Instead, use intptr_t which is guaranted to be castable to (void *)
>> without loss of data.
> Sounds reasonable, glibc syscall returns long but I guess that's because
> there was no intptr_t when that was introduced.
>
>> Signed-off-by: Teo Couprie Diaz <teo.coupriediaz@arm.com>
>> ---
>>   include/lapi/syscalls/regen.sh |   2 +-
>>   runtest/check_tst_syscall      | 190 +++++++++++++++++++++++++++++++++
>>   2 files changed, 191 insertions(+), 1 deletion(-)
>>   create mode 100644 runtest/check_tst_syscall
>>
>> diff --git a/include/lapi/syscalls/regen.sh b/include/lapi/syscalls/regen.sh
>> index 3bf38fd03..97027e2f3 100755
>> --- a/include/lapi/syscalls/regen.sh
>> +++ b/include/lapi/syscalls/regen.sh
>> @@ -48,7 +48,7 @@ cat << EOF > "${output_pid}"
>>   #endif
>>   
>>   #define tst_syscall(NR, ...) ({ \\
>> -	int tst_ret; \\
>> +	intptr_t tst_ret; \\
>>   	if (NR == __LTP__NR_INVALID_SYSCALL) { \\
>>   		errno = ENOSYS; \\
>>   		tst_ret = -1; \\
>> diff --git a/runtest/check_tst_syscall b/runtest/check_tst_syscall
>> new file mode 100644
>> index 000000000..7a6003593
>> --- /dev/null
>> +++ b/runtest/check_tst_syscall
> I do not think that we shoud add this file, at least not in this commit
> and without any good description.

I agree, I wouldn't want to merge it.
As mentioned in the cover, I wanted to share the list of tests I have 
tested the patch with,
both for people to test themselves if they want to and as a way to ask 
if there was anything I missed
while testing with that.
I didn't really know how to share this, so I added it as part of the 
commit for the RFC. Maybe it would
fit better as part of the cover letter ? Or after the commit description 
with the shortlog ?
It might be better removed altogether and people can test with a larger 
scope anyway !

Thanks for having a look,
Best regards
Téo Couprie Diaz


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp