Hello DanC, On Thu, 2012-05-17 at 10:08 +0300, Dan Carpenter wrote: > 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'; > - if (!acceptor_values || !proposer_values) { > - if (tmp1) > - *tmp1 = ','; > - if (tmp2) > - *tmp2 = ','; > - return NULL; > - } Mmmmm, I seem to remember this code being some manner of bugfix when these two NULL checks for IS_TYPE_VALUE_LIST() type login parameters where originally added into iscsi-target code (circa 2004).. Looking at iscsi_check_valuelist_for_support() now in modern usage I think it's fine to go ahead and drop these checks.. > 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++ = ','; <nod>, looks harmless to drop as well. Applied to lio-core, and queuing into for-next. Thanks Dan! --nab -- 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