Am 17.05.2012 09:08, schrieb Dan Carpenter: > Neither "acceptor_values" nor "proposer_values" can be NULL here. > Smatch complains because we are not allowed to pass NULL pointers to > strchr(). > > Also I removed a second later check for "!acceptor_values" because it > gets checked on the next line in the do while condition. > > Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx> > --- > Compile tested only. Please review carefully. > > diff --git a/drivers/target/iscsi/iscsi_target_parameters.c b/drivers/target/iscsi/iscsi_target_parameters.c > index ad3b3c1..ed5241e 100644 > --- a/drivers/target/iscsi/iscsi_target_parameters.c > +++ b/drivers/target/iscsi/iscsi_target_parameters.c > @@ -1037,13 +1037,6 @@ static char *iscsi_check_valuelist_for_support( > tmp2 = strchr(acceptor_values, ','); > if (tmp2) > *tmp2 = '\0'; this looks like strchrnul(). I realy do not know is this supported inside the kernel ? > - if (!acceptor_values || !proposer_values) { > - if (tmp1) > - *tmp1 = ','; > - if (tmp2) > - *tmp2 = ','; > - return NULL; > - } > if (!strcmp(acceptor_values, proposer_values)) { > if (tmp2) > *tmp2 = ','; > @@ -1053,8 +1046,6 @@ static char *iscsi_check_valuelist_for_support( > *tmp2++ = ','; > > acceptor_values = tmp2; > - if (!acceptor_values) > - break; > } while (acceptor_values); > if (tmp1) > *tmp1++ = ','; the changes look reasonable but to be fair the code should have an explanation what it is supposed to do. I do not have the feeling that i did understand the purpose completely. just my 2 cents, wh -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html