Re: type mismatch on cpuset.h

Re: type mismatch on cpuset.h

[Mark Geisert]

[Redirected here from the main mailing list...]

Hi Corinna,

Corinna Vinschen via Cygwin wrote:
> Hi Mark,
> 
> On Mar  6 07:57, Marco Atzeri via Cygwin wrote:
>> Hi,
>>
>> building latest gdal I noticed a type mismatch, that forced me to build
>> with "-fpermissive"
>>
>> on /usr/include/sys/cpuset.h
>>
>>   #define CPU_ALLOC(num)      __builtin_malloc (CPU_ALLOC_SIZE(num))
>>
>>
>> but on
>> https://linux.die.net/man/3/cpu_alloc
>>
>>   cpu_set_t *CPU_ALLOC(int num_cpus)
>>
>>
>> so void* versus cpu_set_t*
> 
> Marco is correct.  cpuset.h was your pet project a while back.  Would
> you like to pick it up?  Maybe we should convert all the macros into
> type-safe inline functions, or macros calling type-safe (inline)
> functions, as on Linux as well as on BSD?

As far as I can tell from online docs, the CPU_SET(3) macros are still macros on 
Linux, though they are documented with prototypes as if they were functions.  I 
don't immediately see a need to change our cpuset.h for this.

I'm also uncertain what exactly you mean by "type-safe" in this context.  Could 
you please give me an example for one of the macros?

I desk-checked all the macros vs their prototypes and I believe CPU_ALLOC that 
Marco ran into is the only faulty one.  It could be fixed with a cast.  CPU_FREE's 
result is void so I should make sure __builtin_free() corresponds. 
CPU_ALLOC_SIZE's result is size_t and I believe the macro is correct as-is because 
it is an expression using untyped integers and sizeof()s, the latter are 
size_t-returning.

The other few macros that return results return int, and those are precisely the 
ones whose inline code uses an int variable to accumulate a result.

If there is some other consideration I'm not seeing, e.g. readability, please let 
me know.  Otherwise I don't really see a need for changes here (modulo casting 
return values properly where needed).

..mark

Re: type mismatch on cpuset.h

[Corinna Vinschen]

Hi Mark,

On Mar  7 00:09, Mark Geisert wrote:
> [Redirected here from the main mailing list...]
> 
> Hi Corinna,
> 
> Corinna Vinschen via Cygwin wrote:
> > Hi Mark,
> > 
> > On Mar  6 07:57, Marco Atzeri via Cygwin wrote:
> > > Hi,
> > > 
> > > building latest gdal I noticed a type mismatch, that forced me to build
> > > with "-fpermissive"
> > > 
> > > on /usr/include/sys/cpuset.h
> > > 
> > >   #define CPU_ALLOC(num)      __builtin_malloc (CPU_ALLOC_SIZE(num))
> > > 
> > > 
> > > but on
> > > https://linux.die.net/man/3/cpu_alloc
> > > 
> > >   cpu_set_t *CPU_ALLOC(int num_cpus)
> > > 
> > > 
> > > so void* versus cpu_set_t*
> > 
> > Marco is correct.  cpuset.h was your pet project a while back.  Would
> > you like to pick it up?  Maybe we should convert all the macros into
> > type-safe inline functions, or macros calling type-safe (inline)
> > functions, as on Linux as well as on BSD?
> 
> As far as I can tell from online docs, the CPU_SET(3) macros are still
> macros on Linux, though they are documented with prototypes as if they were
> functions.
> 
> I don't immediately see a need to change our cpuset.h for this.
> 
> I'm also uncertain what exactly you mean by "type-safe" in this context.
> Could you please give me an example for one of the macros?

Marco gave a good example.  Projects expect to be able to do

   cpuset_t *set = CPU_ALLOC(42);

without warnings, especially if -Werror is set in the project.
However, given the malloc call isn't casted to (cpu_set_t *),
you'll get matching warnings.

Look at FreeBSD:

  #define CPU_ALLOC(_s)  __cpuset_alloc(_s)
  extern cpuset_t *__cpuset_alloc(size_t set_size);

or Linux:

  # define CPU_ALLOC(count) __CPU_ALLOC (count)
  #define __CPU_ALLOC(count) __sched_cpualloc (count)
  extern cpu_set_t *__sched_cpualloc (size_t __count);

This is type-safe, because it redirects the macros to functions
taking typed parameters and returning the correct, expected type.

You can combine this with inline definitions, so you get this
all inside the header:

  #define CPU_ALLOC(_s)  __cpuset_alloc(_s)
  static inline cpuset_t *__cpuset_alloc(size_t _size)
  {
    return (cpuset_t *) __builtin_malloc (CPU_ALLOC_SIZE(num));
  }

> I desk-checked all the macros vs their prototypes and I believe CPU_ALLOC
> that Marco ran into is the only faulty one.  It could be fixed with a cast.
> CPU_FREE's result is void so I should make sure __builtin_free()
> corresponds. CPU_ALLOC_SIZE's result is size_t and I believe the macro is
> correct as-is because it is an expression using untyped integers and
> sizeof()s, the latter are size_t-returning.
> 
> The other few macros that return results return int, and those are precisely
> the ones whose inline code uses an int variable to accumulate a result.
> 
> If there is some other consideration I'm not seeing, e.g. readability,
> please let me know.  Otherwise I don't really see a need for changes here
> (modulo casting return values properly where needed).

That's ok, make your own judgement call.  I don't expect you to
implement this in a certain style, but looking at FreeBSD headers is
often a good idea.


Corinna