Sorry, I knee jerk react to "partial match", which was a red flag for
me, so strncmp() is almost never used for "partial match" in my
experience. :-)
It's really that strncmp() should be used for buffer overflow
protection, but in this case - it's not. Since, the ", 5" applies to
the clearly visible "=All" that is both allocated and '\0' terminated.
It should be against the remaining buffer size of text_ptr. So,
removing the ", 5" leaves it equally as bad at buffer overflow
protection as it was before. :-)
On Thu, Jan 27, 2022 at 10:19 AM Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
>
> We want to match the whole string "=All" and this code does that, but
> strncmp() is normally used for a partial match and it's more readable
> to use strcmp().
>
> Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> ---
> drivers/target/iscsi/iscsi_target.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
> index 2c54c5d8412d..4534101a7376 100644
> --- a/drivers/target/iscsi/iscsi_target.c
> +++ b/drivers/target/iscsi/iscsi_target.c
> @@ -2213,7 +2213,7 @@ iscsit_process_text_cmd(struct iscsi_conn *conn, struct iscsi_cmd *cmd,
> /* '=' confirmed in strncmp */
> text_ptr = strchr(text_in, '=');
> BUG_ON(!text_ptr);
> - if (!strncmp("=All", text_ptr, 5)) {
> + if (!strcmp("=All", text_ptr)) {
> cmd->cmd_flags |= ICF_SENDTARGETS_ALL;
> } else if (!strncmp("=iqn.", text_ptr, 5) ||
> !strncmp("=eui.", text_ptr, 5)) {
> --
> 2.20.1
>
--
Mark Mielke <mark.mielke@xxxxxxxxx>
