Re: [PATCH 1/1] get page size using sysconf (_SC_PAGESIZE) instead of PAGE_SIZE

[Zied Guermazi <zied.guermazi@trande.de>]

hi Markus,

thanks for your feedback, please have a look below for the answers.

On 25.04.22 07:03, Metzger, Markus T wrote:
> Hello Zied,
>
>> diff --git a/gdb/nat/linux-btrace.c b/gdb/nat/linux-btrace.c
>> index b0d6dcd7cf1..a9aaa9c88b3 100644
>> --- a/gdb/nat/linux-btrace.c
>> +++ b/gdb/nat/linux-btrace.c
>> @@ -486,9 +486,10 @@ linux_enable_bts (ptid_t ptid, const struct
>> btrace_config_bts *conf)
>>    if (fd.get () < 0)
>>      diagnose_perf_event_open_fail ();
>>
>> +  long page_size = sysconf (_SC_PAGESIZE);
> Please check the return value.  You may just error out but we should
> detect errors, at least.
[Zied] according to man pages the returned values should not be below 1. 
I will check against it and write a warning message.
>
>
>>    struct perf_event_mmap_page *header = (struct perf_event_mmap_page *)
>>      data.get ();
>> -  data_offset = PAGE_SIZE;
>> +  data_offset = page_size;
> Hmmm, I would have expected a compiler warning, here, since PAGE_SIZE is
> now long and DATA_OFFSET is __u64.

[Zied] I go a clean compilation on my machine. here is the outcome:

"build_intel/gdb$ make
   CXX    nat/linux-btrace.o
   GEN    init.c
   CXXLD  gdb
"

Do you recommend casting page_size to a __u64?

>
>
>>    /* Allocate the configuration page. */
>> -  scoped_mmap data (nullptr, PAGE_SIZE, PROT_READ | PROT_WRITE,
>> MAP_SHARED,
>> +  long page_size = sysconf (_SC_PAGESIZE);
> Here, we'd also want to check the return value.
[Zied] ok
>
>
>> @@ -661,7 +663,7 @@ linux_enable_pt (ptid_t ptid, const struct
>> btrace_config_pt *conf)
>>        size_t length;
>>        __u64 data_size;
>>
>> -      data_size = (__u64) pages * PAGE_SIZE;
>> +      data_size = (__u64) pages * page_size;
> Hmmm, this also looks like the compiler would want to warn about
> signed and unsigned multiply.

[Zied] No warnings observed. Do you recommend casting page_size to a __u64?

>
> Regards,
> Markus.
>
> Intel Deutschland GmbH
> Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
> Tel: +49 89 99 8853-0,www.intel.de  <http://www.intel.de>
> Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva
> Chairperson of the Supervisory Board: Nicole Lau
> Registered Office: Munich
> Commercial Register: Amtsgericht Muenchen HRB 186928
>
-- 
Kind Regards
Zied Guermazi