Re: [RFC] kallsyms_lookup always requires buffers

[Franck Bui-Huu <vagabon.xyz@gmail.com>]

Hi

Paulo Marques wrote:
> Franck Bui-Huu wrote:
>>
>> This simple patch adds a new entry "kallsyms_lookup_gently()". The
>> name actually sucks but I can't figure out a coherent name with the
>> rest of the file. If someone could give me a better idea...
> 
> kallsyms_lookup_size_offset() ?
> 
> Granted it is a little bigger, but it tells exactly what it does and
> there are not that many users that we have to write this name all that
> often.
> 

ok.

>> This new entry does exactly the same as kallsyms_lookup() but does
>> not require any buffers to store any names. It returns 0 if it fails
>> otherwise 1.
>>
>> Do you think this can be usefull ?
> 
> You tell me, since you're proposing the change ;)
> 

:)
actually the question was rather "does this change look sensible to you ?"

>> +static int is_kernel_addr(unsigned long addr)
> 
> Maybe change the name to kallsyms_is_kernel_addr, because this function
> does more things than what the generic name implies.
> 

it sounds like a public function name. Maybe is_ksym_addr() is better ?

>> + * Lookup an address but don't bother to find any names.
>> + */
>> +int kallsyms_lookup_gently(unsigned long addr, unsigned long
>> *symbolsize,
>> +               unsigned long *offset)
>> +{
>> +    int rv;
>> +
>> +    if (is_kernel_addr(addr))
>> +        rv = !!get_symbol_pos(addr, symbolsize, offset);
>> +    else
>> +        rv = !!module_address_lookup(addr, symbolsize, offset, NULL);
>> +
>> +    return rv;
>> +}
> 
> <minor nitpick>
> 
> Why not just:
> 
>> if (is_kernel_addr(addr))
>>    return !!get_symbol_pos(addr, symbolsize, offset);
>>
>> return !!module_address_lookup(addr, symbolsize, offset, NULL);
> 
> and just get rid of "rv" completely?
> 
> </minor nitpick>
> 

No problem.

>> +EXPORT_SYMBOL_GPL(kallsyms_lookup_gently);
> 
> I agree with Arjan here. If kallsyms_lookup wasn't exported, I don't see
> a reason to export this either.
> 

Already removed.

thanks
		Franck