NeilBrown <neilb@xxxxxxx> writes: > On Thu, Mar 10 2016, Jes Sorensen wrote: > >> Guoqing Jiang <gqjiang@xxxxxxxx> writes: >>> On 03/09/2016 01:30 AM, Jes.Sorensen@xxxxxxxxxx wrote: >>>> @@ -297,7 +297,14 @@ int Grow_addbitmap(char *devname, int fd, struct context *c, struct shape *s) >>>> " between different architectures. Consider upgrading the Linux kernel.\n"); >>>> } >>>> - if (s->bitmap_file && strcmp(s->bitmap_file, "clustered") == >>>> 0) >>>> + /* >>>> + * We only ever get called if s->bitmap_file is != NULL, so this check >>>> + * is just here to quiet down static code checkers. >>>> + */ >>>> + if (!s->bitmap_file) >>>> + return 1; >>> >>> Is it really need to make all static code checkers happy? ;-) >>> Otherwise, I would prefer remove above check. >>> >>> Anyway, I am fine with the changes. >> >> We had a check in one place, but not in the remaining places. I just >> made it more consistent. > > I wonder if maybe the checker was only complaining because the code > was inconsistent. i.e. if we just got rid of the existing test on > s->bitmap_file, maybe that would make the checker happy. It would be > interesting to experiment even if you ultimately decide to leave the > new test there. I went back and checked the scan logs and it basically complained about all the cases that didn't check the validity of s->bitmap_file. I think it's safer to just have the one check at the top, even if we expect it never to be called with an invalid pointer. Cheers, 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
