[Ltrace-devel] Getting prototypes from debug information

[Dima Kogan]

New version pushed. Comments inline.


Petr Machata <pmachata at redhat.com> writes:

> You addressed the *-at-variable in function prototypes, but this should
> apply everywhere
>
> Please drop the extra spaces.

I thought I took care of those earlier. In any case, those should be
fixed now.


> +	switch (byte_size) {
> +	case sizeof(int):
> +		*type = is_signed ? ARGTYPE_INT : ARGTYPE_UINT;
> +		return true;
> +
> +	case sizeof(long):
> +		*type = is_signed ? ARGTYPE_LONG : ARGTYPE_ULONG;
> +		return true;
> +	}
> +}
>
> This won't work on 32-bit arches, where int and long are both 4 bytes
> wide

I thought I took care of this too; weird. Fixed also.


> +	while ((die = dwfl_nextcu(dwfl, die, &bias)) != NULL) {
>
> This doesn't seem right.  dwfl_nextcu should go through all CU's of the
> whole Dwfl--i.e. if you add two Dwarf-based libraries to a process, it
> would iterate the former one twice.  Or maybe I don't quite grasp how
> your code works.

I'm not trying to do anything noteworthy here; just iterating through
all the compile units. You're saying that if a process that uses two
libraries, this could would look through those libraries twice? I don't
understand the issue you're describing.


> Is there a reason not to just use uint64_t for encoding?

Nope. Typo.


> Just a tip: %#02x includes the initial 0x.

Good tip! I'm leaving the current code alone to reduce churn, but
definitely will use that in the future.


>
> +		type_struct_add(result, member_type,
> +				newly_allocated_member_type);
>
> This can fail.
>
> +		dict_insert(type_dieoffset_hash, &die_offset, &result);
>
> This as well.

I added more error checking.