Hi Cristian,
* Cristian Rodríguez <crrodriguez@xxxxxxxxxxxx> [2011-12-16 15:27:48 -0300]:
> - Reduce the scope of i, child_count
> - Fd leak on failure
> - Initialize st
> ---
> libkmod/libkmod-index.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/libkmod/libkmod-index.c b/libkmod/libkmod-index.c
> index 7f0c092..c376176 100644
> --- a/libkmod/libkmod-index.c
> +++ b/libkmod/libkmod-index.c
> @@ -258,7 +258,6 @@ static struct index_node_f *index_read(FILE *in, uint32_t offset)
> {
> struct index_node_f *node;
> char *prefix;
> - int i, child_count = 0;
>
> if ((offset & INDEX_NODE_MASK) == 0)
> return NULL;
> @@ -276,6 +275,7 @@ static struct index_node_f *index_read(FILE *in, uint32_t offset)
> if (offset & INDEX_NODE_CHILDS) {
> char first = read_char(in);
> char last = read_char(in);
> + int i, child_count = 0;
> child_count = last - first + 1;
>
> node = NOFAIL(malloc(sizeof(struct index_node_f) +
> @@ -343,6 +343,7 @@ struct index_file *index_file_open(const char *filename)
>
> magic = read_long(file);
> if (magic != INDEX_MAGIC)
> + fclose(file);
> return NULL;
You should enclose the if with braces.
>
> version = read_long(file);
> @@ -724,7 +725,7 @@ struct index_mm *index_mm_open(struct kmod_ctx *ctx, const char *filename,
> uint32_t root_offset;
> } hdr;
> void *p;
> -
> + memset(&st, 0, sizeof(st));
Whatever static analyzer you're using, probably it's getting this wrong,
or the fix is the wrong one.
st is initialized in the call to fstat(). If this is going to be fixed,
we should check the return value of fstat() and handle it as an error
path. However none of the error cases of fstat() are appealing to me:
EBADF we doesn't need to treat since we just opened the file. Treating
EIO and EOVERFLOW I'm not sure, but should be done as in the read_safe_*
functions.
> DBG(ctx, "file=%s\n", filename);
>
> idx = malloc(sizeof(*idx));
> @@ -737,7 +738,6 @@ struct index_mm *index_mm_open(struct kmod_ctx *ctx, const char *filename,
> ERR(ctx, "%m\n");
> goto fail_open;
> }
> -
Unrelated line and against the coding style.
> fstat(fd, &st);
> flags = MAP_PRIVATE;
> if (populate)
> --
Please split the patch for different fixes too.
Thanks
Lucas De Marchi
--
To unsubscribe from this list: send the line "unsubscribe linux-modules" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
