On Fri, Apr 07, 2017 at 10:08:35AM +0200, Erik Skultety wrote:
> > +#include <config.h>
> > +
> > +#include "virmock.h"
> > +#include "virfilemock.h"
>
> ^^These are local, could you group them with the rest below the system ones?
> (This was pointed out to me during review once or twice).
>
> > +
> > +#include <stdio.h>
> > +#include <stdlib.h>
> ^^These get pulled in by virmock.h, but it's always nice to be explicit about
> being able to use "printf" :P.
>
> > +#include <unistd.h>
>
> I've successfully built the patch without ^^this header. You can drop it.
>
> > +#include <fcntl.h>
>
> ...
>
> > +#include <sys/stat.h>
> > +#include <sys/types.h>
>
> It built without ^^these 2 as well.
>
> > +#include <dirent.h>
>
> ^This one gets pulled in by virfile.h
>
> > +
> > +#include "viralloc.h"
> > +#include "virstring.h"
> > +#include "virfile.h"
> > +
> > +
> > +/* Mapping for prefix overrides */
> > +static size_t noverrides;
> > +static const char **overrides;
> > +
> > +/* nprefixes == noverrides, but two variables make it easier to use
> > + * VIR_*_ELEMENT macros */
> > +static size_t nprefixes;
> > +static const char **prefixes;
> > +
> > +/* TODO: callbacks */
>
> Just out of curiosity, what callbacks?
>
> [...]
>
> > +
> > +
> > +void
> > +virFileMockRemovePrefix(const char *prefix)
> > +{
> > + size_t i = 0;
> > +
> > + for (i = 0; i < noverrides; i++) {
> > + if (STREQ(prefixes[i], prefix))
>
> Since you're removing a single element, you can just delete the prefix here and
> then break from the loop.
>
> > + break;
> > + }
> > +
> > + if (i == noverrides)
> > + return;
>
> You won't be needing ^^this then.
>
> > +
> > + VIR_DELETE_ELEMENT(overrides, i, noverrides);
> > + VIR_DELETE_ELEMENT(prefixes, i, nprefixes);
> > +}
> > +
>
> ACK with the adjustments (with the exception if it would somehow break BSD
> again :)).
You'll also need to add virfilemock.h to tests/Makefile.am EXTRA_DIST otherwise
make distcheck fails with "No such file or directory".
Erik
>
> Erik
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
