[PATCH v3 1/5] dyxlat: building xlat dynamically

[Masatake YAMATO]

On Wed, 14 Jun 2017 00:55:03 +0300, "Dmitry V. Levin" <ldv at altlinux.org> wrote:
> On Tue, Jun 13, 2017 at 05:26:41PM +0900, Masatake YAMATO wrote:
>> xlat tables are generating in built-time of strace.
>> 
>> printxval is suitable for printing `family' field of Netlink GENERIC
>> protocol. However, contents of the xlat table cannot be defined in
>> build-time because the values for the field are given by Linux kernel.
>> 
>> dyxlat functions are for building xlat in run-time. Decoding the
>> field is the primary use case of functions but they can be used
>> another purpose.
>> 
>> * defs.h (dyxlat_alloc, dyxlat_free, dyxlat_add_pair): New functions
>> declarations.
>> (struct dyxlat): New opaque data type.
>> 
>> * dyxlat.c: New file.
>> 
>> Changes in version 3 (suggested by ldv):
>> 
>> 	* Rename dyxlat_new/dyxlat_delete to dyxlat_alloc/dyxlat_free.
>> 	  Rename dyxlat_may_add_pair to dyxlat_may_add_pair.
>> 
>> 	* Move dyxlat declarations closer to other xlat related prototypes.
>> 
>> 	* Change the field types of dyxlat structure from int to size_t.
>> 
>> 	* Merge the file static functions in dyxlat.c to the exported functions
>> 	  in the file.
>> 
>> Signed-off-by: Masatake YAMATO <yamato at redhat.com>
>> ---
>>  Makefile.am |   1 +
>>  defs.h      |   6 ++++
>>  dyxlat.c    | 103 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 110 insertions(+)
>>  create mode 100644 dyxlat.c
>> 
>> diff --git a/Makefile.am b/Makefile.am
>> index 80f8a34..d004ac6 100644
>> --- a/Makefile.am
>> +++ b/Makefile.am
>> @@ -104,6 +104,7 @@ strace_SOURCES =	\
>>  	dirent.c	\
>>  	dirent64.c	\
>>  	dm.c		\
>> +	dyxlat.c	\
>>  	empty.h		\
>>  	epoll.c		\
>>  	evdev.c		\
>> diff --git a/defs.h b/defs.h
>> index 6449bce..52f40b3 100644
>> --- a/defs.h
>> +++ b/defs.h
>> @@ -504,6 +504,12 @@ extern enum sock_proto getfdproto(struct tcb *, int);
>>  extern const char *xlookup(const struct xlat *, const uint64_t);
>>  extern const char *xlat_search(const struct xlat *, const size_t, const uint64_t);
>>  
>> +struct dyxlat;
>> +struct dyxlat *dyxlat_alloc(size_t allocation);
> 
> as "allocation" means number of elements (as nmemb argument of calloc),
> let's call it "nmemb" or something like that.
> 
>> +void dyxlat_free(struct dyxlat *dyxlat);
> 
> Do you think "struct dyxlat *dyxlat" is more descriptive
> than "struct dyxlat *"?
> 
>> +struct xlat *dyxlat_get(struct dyxlat *dyxlat);
> 
> This function doesn't change anything, let's change its prototype to
> const struct xlat *dyxlat_get(const struct dyxlat *);
> 
>> +void dyxlat_add_pair(struct dyxlat *dyxlat, uint64_t val, const char *str);
> 
> It looks like in the primary use case (genl_parse_families_response)
> it would be convenient to specify the string length as well.
> 
> I suggest changing this prototype to
> void dyxlat_add_pair(struct dyxlat *, uint64_t val, const char *str, size_t len);
> 
> The implementation would invoke xstrndup instead of xstrdup
> if len is not equal to zero.

Do you have any reason not to use xstrndup if len is equal to zero?
I cannot find the reason. I think I can use xstrndup regardless of len.

Masatake YAMATO