On Mon, 2 Apr 2012 21:58:52 +0200
Arne Redlich <arne.redlich@xxxxxxxxxxxxxx> wrote:
> 2012/4/2 FUJITA Tomonori <fujita.tomonori@xxxxxxxxxxxxx>:
>> On Sat, 31 Mar 2012 23:56:00 +0200
>> Arne Redlich <arne.redlich@xxxxxxxxxxxxxx> wrote:
>>
>>> The get_hdr_param macro is used exuberantly, leading to variables
>>> that are defined and set but never actually used, as pointed out by
>>> gcc-4.6.
>>>
>>> Signed-off-by: Arne Redlich <arne.redlich@xxxxxxxxxxxxxx>
>>> ---
>>>
>>> Tomo, list,
>>>
>>> This patch is a rather mechanic translation from IET's isns code, and merely compile tested.
>>>
>>> Cheers,
>>> Arne
>>>
>>> usr/iscsi/isns.c | 20 ++++++++------------
>>> 1 files changed, 8 insertions(+), 12 deletions(-)
>>>
>>>
>>> diff --git a/usr/iscsi/isns.c b/usr/iscsi/isns.c
>>> index 1f1852c..452c7af 100644
>>> --- a/usr/iscsi/isns.c
>>> +++ b/usr/iscsi/isns.c
>>> @@ -638,12 +638,10 @@ static int recv_pdu(int fd, struct isns_io *rx, struct isns_hdr *hdr)
>>> static char *print_scn_pdu(struct isns_hdr *hdr)
>>> {
>>> struct isns_tlv *tlv = (struct isns_tlv *) hdr->pdu;
>>> - uint16_t function, length, flags, transaction, sequence;
>>> + uint16_t length = ntohs(hdr->length);
>>> char *name = NULL;
>>> static char iscsi_name[224];
>>>
>>> - get_hdr_param(hdr, function, length, flags, transaction, sequence);
>>
>> I think that the point of get_hdr_param is putting how to get extract
>> values from a header into one place. That is, I like to avoid putting
>> ntohs everywhere.
>>
>> I guess we had better to drop -Wall?
>
> I don't think this is a good idea for any project as -Wall (and
> various other "-W"s) has (have) proven to be quite invaluable to me.
I would agree in general but I'm not sure "-Wunused-but-set-variable"
warning is useful.
> If you think abstractions are useful there (I'm not convinced, as it's
> all internal to isns.c), how about getters per header field?
Ok, just put dprintf in get_hdr_param macro.
diff --git a/usr/iscsi/isns.c b/usr/iscsi/isns.c
index 1f1852c..a621ef0 100644
--- a/usr/iscsi/isns.c
+++ b/usr/iscsi/isns.c
@@ -578,6 +578,8 @@ static int recv_hdr(int fd, struct isns_io *rx, struct isns_hdr *hdr)
flags = ntohs(hdr->flags); \
transaction = ntohs(hdr->transaction); \
sequence = ntohs(hdr->sequence); \
+ dprintf("got a header %x %u %x %u %u\n", function, length, flags, \
+ transaction, sequence); \
}
static int recv_pdu(int fd, struct isns_io *rx, struct isns_hdr *hdr)
@@ -591,8 +593,6 @@ static int recv_pdu(int fd, struct isns_io *rx, struct isns_hdr *hdr)
/* Now we got a complete header */
get_hdr_param(hdr, function, length, flags, transaction, sequence);
- dprintf("got a header %x %u %x %u %u\n", function, length, flags,
- transaction, sequence);
if (length + sizeof(*hdr) > BUFSIZE) {
eprintf("FIXME we cannot handle this yet %u!\n", length);
--
To unsubscribe from this list: send the line "unsubscribe stgt" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
