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

[Dmitry V. Levin]

On Sat, Jun 17, 2017 at 03:48:06AM +0900, Masatake YAMATO wrote:
> 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.

I don't remember, probably I meant passing zero length instead of strlen
to dyxlat_add_pair.  It's fine to invoke xstrndup unconditionally.


-- 
ldv
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20170616/2511985e/attachment.bin>