On Tue, Oct 02, 2012 at 01:48:57PM +0200, walter harms wrote:
>
>
> Am 02.10.2012 10:22, schrieb Dan Carpenter:
> > Clang warns about this bug:
> > drivers/target/iscsi/iscsi_target_erl0.c:52:45: warning: operator '?:'
> > has lower precedence than '+'; '+' will be evaluated first
> > [-Wparentheses]
> >
> > Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> > ---
> > Please review this very carefully because I haven't tested it. It could
> > be that the check should be:
> > (data_done + data_length > FirstBurstLength) ? FirstBurstLength : data_length);
> > Instead of what I have which is:
> > data_done + (data_length > FirstBurstLength ? FirstBurstLength : data_length);
> >
> > diff --git a/drivers/target/iscsi/iscsi_target_erl0.c b/drivers/target/iscsi/iscsi_target_erl0.c
> > index 1a02016..2067efd 100644
> > --- a/drivers/target/iscsi/iscsi_target_erl0.c
> > +++ b/drivers/target/iscsi/iscsi_target_erl0.c
> > @@ -48,9 +48,9 @@ void iscsit_set_dataout_sequence_values(
> > if (cmd->unsolicited_data) {
> > cmd->seq_start_offset = cmd->write_data_done;
> > cmd->seq_end_offset = (cmd->write_data_done +
> > - (cmd->se_cmd.data_length >
> > - conn->sess->sess_ops->FirstBurstLength) ?
> > - conn->sess->sess_ops->FirstBurstLength : cmd->se_cmd.data_length);
> > + ((cmd->se_cmd.data_length >
> > + conn->sess->sess_ops->FirstBurstLength) ?
> > + conn->sess->sess_ops->FirstBurstLength : cmd->se_cmd.data_length));
> > return;
> > }
> >
>
> the ?: operator is nice but at a certain length is starts to become unreadable,
> the end is normally calculated by end= start+len; Therefor i suggest:
>
> if ( cmd->se_cmd.data_length > conn->sess->sess_ops->FirstBurstLength )
> cmd->seq_end_offset = cmd->seq_start_offset + conn->sess->sess_ops->FirstBurstLength;
> else
> cmd->seq_end_offset = cmd->seq_start_offset + cmd->se_cmd.data_length;
>
Yeah. It's not beautiful.
Doing a "struct iscsi_sess_ops *ops = conn->sess->sess_ops;" at the
start of the function would help as well.
regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
