[CRIU] [PATCH 2/2] vdso: x86 -- Use dynamic symbols for parsing

[Pavel Emelyanov]

On 05/28/2014 01:26 AM, Cyrill Gorcunov wrote:
> On Wed, May 28, 2014 at 12:55:50AM +0400, Pavel Emelyanov wrote:
>> On 05/26/2014 03:02 PM, Cyrill Gorcunov wrote:
>>> New vDSO are in stripped format so use dynamic
>>> symbols instead of sectioned ones.
>>
>> How about parasite-based vdso page detection? Has it changed?
> 
> Nope. Only symbols parsing is changed.

ok

>>> -#define DECLARE_VDSO(ident_name, symtab_name)					\
>>
>> Do we need the macro itself?
> 
> Not anymore.

then remove one as well

>>> +static unsigned long elf_hash(const unsigned char *name)
>>> +{
>>> +	unsigned long h = 0, g;
>>> +
>>> +	while (*name) {
>>> +		h = (h << 4) + *name++;
>>> +		g = h & 0xf0000000ul;
>>> +		if (g)
>>> +			h ^= g >> 24;
>>> +		h &= ~g;
>>> +	}
>>> +	return h;
>>
>> Where did this come from?
> 
> This hash from elf specification.

add a comment about it

>>> +	/*
>>> +	 * See Elf specification.
>>
>> For what?
> 
> To decode the "magic" values if one is interested.

write this in comment

>>> +	 */
>>> +	const char elf_ident[] = {
>>> +		0x7f, 0x45, 0x4c, 0x46, 0x02, 0x01, 0x01, 0x00,
>>> +		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>>> +	};
>>>  
>>> -	DECLARE_VDSO(vdso_ident, vdso_symbols);
>>> +	const char *symtab_name[VDSO_SYMBOL_MAX] = {
>>> +		[VDSO_SYMBOL_CLOCK_GETTIME]	= VDSO_SYMBOL_CLOCK_GETTIME_NAME,
>>> +		[VDSO_SYMBOL_GETCPU]		= VDSO_SYMBOL_GETCPU_NAME,
>>> +		[VDSO_SYMBOL_GETTIMEOFDAY]	= VDSO_SYMBOL_GETTIMEOFDAY_NAME,
>>> +		[VDSO_SYMBOL_TIME]		= VDSO_SYMBOL_TIME_NAME,
>>> +	};
>>
>> These two are moves from header. Can we split the patch?
> 
> Could you please be more detailed here? What exactly you wanted me to split?

patch 1/2 -- move stuff around and rename
patch 2/2 -- use dynamic ...

>>> +
>>>  	/*
>>> -	 * Where the section names lays.
>>> +	 * Dynamic section should provide us the rest of
>>
>> Dynamic _sections_? Or symbols?
> 
> This is a special section called "dynamic" -- it's section name, it consists
> of dynamic names ;)

ah, ok

>>> +
>>> +#if 0
>>
>> Trash?
> 
> no no, I use it for debug -- I can't allow it in pie mode because compiler
> does weird tricks and I may end up in segfault. so lets leave it here, I
> might need it next time we need to modify this code.

why not just leave it under pr_debug, it's exactly for that?