Re: [PATCH v2 04/12] tests: Add virfilemock -- the new super mock

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> +#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 :)).

Erik

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux