"John Stoffel" <john@xxxxxxxxxxx> writes:
>>>>>> "Jes" == Jes Sorensen <Jes.Sorensen@xxxxxxxxxx> writes:
>
> Jes> From: Jes Sorensen <Jes.Sorensen@xxxxxxxxxx>
> Jes> Signed-off-by: Jes Sorensen <Jes.Sorensen@xxxxxxxxxx>
> Jes> ---
> Jes> Incremental.c | 2 +-
> Jes> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> Jes> diff --git a/Incremental.c b/Incremental.c
> Jes> index 87d9114..33c0d7f 100644
> Jes> --- a/Incremental.c
> Jes> +++ b/Incremental.c
> Jes> @@ -1354,7 +1354,7 @@ restart:
> Jes> if (st && st->ss->load_container)
> Jes> ret = st->ss->load_container(st, mdfd, NULL);
> Jes> close(mdfd);
> Jes> - if (!ret && st->ss->container_content) {
> Jes> + if (!ret && st && st->ss->container_content) {
> Jes> if (map_lock(&map))
> Jes> pr_err("failed to get exclusive lock on mapfile\n");
> Jes> ret = Incremental_container(st, me->path, c, only);
> Jes> --
> Jes> 2.1.0
>
> Forgive my stupidity, but how does this really help anything? You
> already did the check above for a valid 'st', and now you're just
> repeating it. Maybe if needs to be more of:
>
> if (st) {
> if (st->ss->load_container)
> ret = st->ss->load_container(st,mdfd, NULL);
> close(mdfd);
> if (!ret && st->ss->container_content) {
> .....
> }
> }
>
> but maybe I'm just missing something here.
Please look more carefully - the checks above are in place so 'st' is
only dereferenced if 'st is valid. The code does not bail out if
st = NULL.
Your example results in mdfd not getting closed if st == NULL.
Jes
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
