Re: [PATCH] dissect: use built_in_ident() instead of MK_IDENT()

[Oleg Nesterov <oleg@redhat.com>]

On 06/20, Luc Van Oostenryck wrote:
>
> On Tue, Jun 20, 2017 at 01:17:41PM +0200, Oleg Nesterov wrote:
> > On 06/19, Luc Van Oostenryck wrote:
> > >
> > > The motivation for this patch was to allow sparse to be
> > > compiled with clang which doesn't like what is done
> > > on VLAs in the MK_IDENT() macro.
> >
> > could you spell please? I don't understand why clang can't compile this
> > code and I am just curious.
>
> Sure, I should have added in the commit message. Here it is:
> 	test-dissect.c:50:16: warning: field 'ident' with variable sized type 'struct ident' not at the end of a struct or class is a GNU extension [-Wgnu-variable-sized-type-not-at-end]
> 	                sym->ident = MK_IDENT("__asm__");
> 	                             ^
> 	./dissect.h:31:16: note: expanded from macro 'MK_IDENT'
> 	                struct ident ident;                     \
> 	                             ^
> 	test-dissect.c:50:25: error: initialization of flexible array member is not allowed
> 	                sym->ident = MK_IDENT("__asm__");
> 	                                      ^~~~~~~~~
> 	./dissect.h:35:11: note: expanded from macro 'MK_IDENT'
> 	                .name = s,                              \
> 	                        ^

Hmm. Now that I git-cloned the code I understand. And I can't recall why MK_IDENT() was
written this way, it simply doesn't look right. Probably can be fixed but I agree, lets
remove it.

> > > But also, I can't see any justification for not using the
> > > real thing to create identifiers: built_in_ident().
> >
> > Simply because MK_IDENT() is faster and simpler to use.
>
> I find this very weak. For the simplicity of use, in both cases
> there is just a single call with the same argument, for the speed,
> I really really doubt there is any measurable difference.

Yes, I didn't try to convince you this optimization actually makes sense,
just explained the motivation I (probably) had many years ago.

> > >  dissect.h      | 13 -------------
> > >  test-dissect.c |  6 +++---
> > 
> > iirc there should be another MK_IDENT user in dissect.c?
> 
> No, it doesn't seem to, and the history tell that there it
> never had been the case, just the three in test-dissect.c

OK, my memory fooled me.

Thanks,

Oleg.