On 20.03.2020 16:43, Tom Tromey wrote:
>>>>>> "Kamil" == Kamil Rytarowski <n54@gmx.com> writes:
> 
> Kamil> Register nbsd_auxv_parse() that overloads the default (Linux-style)
> Kamil> AUXV parsing. On NetBSD the type parameter is defined as int32_t
> Kamil> for all architectures.
> 
> Thanks for the patch.
> 
> Kamil> +
> Kamil> +static int
> Kamil> +nbsd_auxv_parse (struct gdbarch *gdbarch, gdb_byte **readptr,
> Kamil> +		 gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp)
> 
> In gdb a new function should always have an intro comment.  This one
> could just explain which gdbarch callback it implements.
> 

Done.

> Kamil> +{
> Kamil> +  struct type *int_type = builtin_type (gdbarch)->builtin_int;
> 
> Presumably you want builtin_in32 here?  builtin_int is
> architecture-dependent.  Or you could just hard-code the size, with a
> suitable comment.
> 

int is de facto alias for int32 on NetBSD.

It cannot be shorter (there are assumptions that it is at least 32-bit)
and a lot of things will break for making it longer. So it will likely
stay that way forever.

I prefer to keep it as 'int' to match the preexisting OpenBSD code.

> Kamil> +
> Kamil> +void
> Kamil> +nbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
> 
> The intro comment here should probably say "See nbsd-tdep.h."...
> 

Done.

> Kamil> +++ b/gdb/nbsd-tdep.h
> Kamil> @@ -25,4 +25,6 @@ struct link_map_offsets *nbsd_lp64_solib_svr4_fetch_link_map_offsets (void);
> 
> Kamil>  int nbsd_pc_in_sigtramp (CORE_ADDR, const char *);
> 
> Kamil> +void nbsd_init_abi (struct gdbarch_info, struct gdbarch *);
> 
> ... and then there should be a descriptive comment above this line.
> 

Done.

> thanks,
> Tom
> 

Please see v3.