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
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
