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).
Yeah, I forgot to get to the header cleaning part. Most of this was
copy-paste from cgroupmock and then I just added my new header.
> +
> +#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?
See commit message. I don't like repeating myself. Or should I copy
the commit message to the comment?
[...]
> +
> +
> +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.
Yeah, thta was the first variant I went with, but then I didn't like the
extra indentation level, so I changed it to this. Can we get a hacking
file section talking about this? I don't like repainting the shed every
new series.
> + 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 :)).
I'll try to check this one (and the following ones as well) on BSD
before pushing. Hopefully I'll manage to install the machine before
going home.
Erik