Re: [PATCH v2 03/13] powerpc: Remove func_descr_t

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Excerpts from Christophe Leroy's message of October 15, 2021 3:19 pm:
> 
> 
> Le 15/10/2021 à 00:17, Daniel Axtens a écrit :
>> Christophe Leroy <christophe.leroy@xxxxxxxxxx> writes:
>> 
>>> 'func_descr_t' is redundant with 'struct ppc64_opd_entry'
>> 
>> So, if I understand the overall direction of the series, you're
>> consolidating powerpc around one single type for function descriptors,
>> and then you're creating a generic typedef so that generic code can
>> always do ((func_desc_t)x)->addr to get the address of a function out of
>> a function descriptor regardless of arch. (And regardless of whether the
>> arch uses function descriptors or not.)
> 
> An architecture not using function descriptors won't do much with 
> ((func_desc_t *)x)->addr. This is just done to allow building stuff 
> regardless.
> 
> I prefer something like
> 
> 	if (have_function_descriptors())
> 		addr = (func_desc_t *)ptr)->addr;
> 	else
> 		addr = ptr;

If you make a generic data type for architectures without function 
descriptors as such

typedef struct func_desc {
    char addr[0];
} func_desc_t;

Then you can do that with no if. The downside is your addr has to be 
char * and it's maybe not helpful to be so "clever".

>>   - why pick ppc64_opd_entry over func_descr_t?
> 
> Good question. At the begining it was because it was in UAPI headers, 
> and also because it was the one used in our 
> dereference_function_descriptor().
> 
> But at the end maybe that's not the more logical choice. I need to look 
> a bit more.

I would prefer the func_descr_t (with 'toc' and 'env') if you're going 
to change it.

Thanks,
Nick




[Index of Archives]     [Linux Kernel]     [Kernel Newbies]     [x86 Platform Driver]     [Netdev]     [Linux Wireless]     [Netfilter]     [Bugtraq]     [Linux Filesystems]     [Yosemite Discussion]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]

  Powered by Linux