Re: [PATCH v2 11/11] tests/9pfs: add local Tmkdir test

[Greg Kurz]

On Fri, 2 Oct 2020 13:56:14 +0100
Daniel P. Berrangé <berrange@redhat.com> wrote:

> On Fri, Oct 02, 2020 at 01:51:54PM +0200, Christian Schoenebeck wrote:
> > This test case uses the 9pfs 'local' driver to create a directory
> > and then checks if the expected directory was actually created
> > (as real directory) on host side.
> > 
> > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > ---
> >  tests/qtest/virtio-9p-test.c | 139 +++++++++++++++++++++++++++++++++++
> >  1 file changed, 139 insertions(+)
> > 
> > diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
> > index af7e169d3a..93161a4b35 100644
> > --- a/tests/qtest/virtio-9p-test.c
> > +++ b/tests/qtest/virtio-9p-test.c
> > @@ -18,6 +18,62 @@
> >  #define QVIRTIO_9P_TIMEOUT_US (10 * 1000 * 1000)
> >  static QGuestAllocator *alloc;
> >  
> > +/*
> > + * Used to auto generate new fids. Start with arbitrary high value to avoid
> > + * collision with hard coded fids in basic test code.
> > + */
> > +static uint32_t fid_generator = 1000;
> > +
> > +static uint32_t genfid(void)
> > +{
> > +    return fid_generator++;
> > +}
> > +
> > +/**
> > + * Splits the @a in string by @a delim into individual (non empty) strings
> > + * and outputs them to @a out. The output array @a out is NULL terminated.
> > + *
> > + * Output array @a out must be freed by calling split_free().
> > + *
> > + * @returns number of individual elements in output array @a out (without 
> > the
> > + *          final NULL terminating element)
> > + */
> > +static int split(const char *in, const char *delim, char ***out)
> > +{
> > +    int n = 0, i = 0;
> > +    char *tmp, *p;
> > +
> > +    tmp = g_strdup(in);
> > +    for (p = strtok(tmp, delim); p != NULL; p = strtok(NULL, delim)) {
> > +        if (strlen(p) > 0) {
> > +            ++n;
> > +        }
> > +    }
> > +    g_free(tmp);
> > +
> > +    *out = g_malloc0(n * sizeof(char *) + 1); /* last element NULL 
> > delimiter */
> 
> Surely this should be  (n + 1) * sizeof(char *), because the last
> element still needs to be large enough to hold a pointer, not a
> single extra byte.
> 

If you decide to keep this split() function, maybe use g_new0(char *, n + 1) ?
This buys you the math and does type checking as an extra.

> > +
> > +    tmp = g_strdup(in);
> > +    for (p = strtok(tmp, delim); p != NULL; p = strtok(NULL, delim)) {
> > +        if (strlen(p) > 0) {
> > +            (*out)[i++] = g_strdup(p);
> > +        }
> > +    }
> > +    g_free(tmp);
> > +
> > +    return n;
> > +}
> 
> This seems to largely re-invent g_strsplit 
> 
> https://developer.gnome.org/glib/2.62/glib-String-Utility-Functions.html#g-strsplit
> 
> > +
> > +static void split_free(char ***out)
> > +{
> > +    int i;
> > +    for (i = 0; (*out)[i]; ++i) {
> > +        g_free((*out)[i]);
> > +    }
> > +    g_free(*out);
> > +    *out = NULL;
> > +}
> 
> And g_strfreev
> 
> 
> 
> Regards,
> Daniel