On Tue, Apr 10, 2018 at 03:03:06PM -0700, Randy Dunlap wrote:
> From: Randy Dunlap <rdunlap@xxxxxxxxxxxxx>
> with help from Linus. (many moons ago)
>
> sparse addition to print all compound/composite global data symbols
> with their sizes and alignment.
>
> ...
>
> I think that I have incorporated the previous feedback from Luc and Chris.
I don't remember the details but it looks good but for a few details.
>
> +static void list_compound_symbols(struct symbol_list *list)
> +{
> + struct symbol *sym;
> +
> + FOR_EACH_PTR(list, sym) {
> + /* Only show arrays, structures, unions */
> + if (!(sym->namespace & (NS_STRUCT | NS_TYPEDEF | NS_SYMBOL)))
> + continue;
I don't think this is needed: you should only have NS_SYMBOLs here
(NS_STRUCT & NS_TYPEDEF are there for the namespace of structure tag &
typedef name. They are used during parsing but all the types belong
to NS_SYMBOL).
> + if (sym->type != SYM_NODE)
> + continue;
I don't think there is any kind of guarantee that a compound type is always
'surrounded' by a SYM_NODE (and I don't think they should be). So, I think
that the two lines here above should be removed.
> + if (!sym->ctype.base_type)
> + continue;
> + if (is_func_type(sym))
> + continue;
> + if (is_float_type(sym) || is_scalar_type(sym))
> + continue;
> + if (sym->ctype.base_type->type == SYM_BASETYPE)
> + continue;
I think these two lines are redundant with the two just before
(given the SYM_NODE). I also think that it should be better to
move these tests to an helper is_compound_type().
Even better, if you're only interested in structs, unions & arrays,
would be to do something more explicit, for example, like:
if (sym->type == SYM_NODE)
base = sym->ctype.base_type;
else
base = sym;
switch (base->type) {
case SYM_STRUCT: case SYM_UNION: case SYM_ARRAY:
break;
default:
continue;
}
Cheers,
-- Luc
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
