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

Re: [libvirt] [PATCH v3 1/4] iptablesFormatNetwork(): constify target of "netaddr" parameter



On 09/24/13 18:25, Eric Blake wrote:
> On 09/24/2013 07:33 AM, Laszlo Ersek wrote:
>>>
>>> I suspect that forbidding const.*Ptr in "make syntax-check" wouldn't be
>>> a bad idea.
>>
>> The pattern should probably require some whitespace in the middle as
>> well, if constWhateverPtr typedefs are to be accepted as valid.
> 
> Here's the proposed syntax check:
> 
> diff --git i/cfg.mk w/cfg.mk
> index dad8a90..6a17d43 100644
> --- i/cfg.mk
> +++ w/cfg.mk
> @@ -468,6 +468,14 @@ sc_correct_id_types:
>  	halt="use pid_t for pid, uid_t for uid, gid_t for gid"		\
>  	  $(_sc_search_regexp)
> 
> +# 'const fooPtr a' is the same as 'foo * const a', even though it is
> +# usually desired to have 'foo const *a'.  It's easier to just prevent
> +# the confusing mix of typedef vs. const placement.
> +sc_forbid_const_pointer_typedef:
> +	@prohibit='const [a-zA-Z_0-9]*Ptr'				\
> +	halt="'const fooPtr var' does not declare what you meant"	\
> +	  $(_sc_search_regexp)
> +
>  # Forbid sizeof foo or sizeof (foo), require sizeof(foo)
>  sc_size_of_brackets:
>  	@prohibit='sizeof\s'						\
> 
> 
> and here's the damage we'd have to clean up:
> 
> $ make sc_forbid_const_pointer_typedef | wc
> maint.mk: 'const fooPtr var' does not declare what you meant
> make: *** [sc_forbid_const_pointer_typedef] Error 1
>     403    1766   37136
> 
> spread among 75 files.
> 
> There's probably some fallout, too - once you have a const-correct
> pointer type, it might show us places where we have been assigning
> through what we thought was const.

Likely not actively assigning (which would be undefined behavior of
coure), just perhaps casting away real constness (which itself is not
undefined behavior). But gcc would "help" with those.

I'm not a convincing stakeholder in libvirt (yet! :) ), so I'm not
suggesting to go ahead with it. But the pattern looks good (assuming you
have an LC_ALL=C setting somewhere in the checker, since regex ranges
depend on collation order, but I'm 100% sure I don't have to say that,
and the setting is already there.)

Thanks,
laszlo


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