[Patch 1/1] Segfault in ietd buffer overrun in dump_line

[Shreyansh]

Hi List,

As discussed in this thread, following (updated with comments) patch
fixes a buffer overrun problem in __dump_line function in log.c file.
It also removes a stray 'return' call in log_pdu function which was
restricting a PDU dump call to complete log_pdu operation.

In dump_line, the array line[] was defined as line[53] whereas sprintf
was pushing in 59 characters into it. This was leading to segfault of
ietd when debugging was enabled. Through this patch, the size of line
char array would be linked to the size of buffer defined in log_pdu
function. The calculation used is (N*3 + ((N/4)+1)*2 + 3), where N is
the expected size of buffer used in log_pdu function.

Also, in log_pdu function, the first check for log_level against
passed parameter for log was incorrect and would have forced a return
even when the caller has set the logging level to acceptable level.

Signed-off-by: Shreyansh Jain <shrey.linux at gmail.com>

---BEGIN---
Index: usr/log.c
===================================================================
--- usr/log.c (revision 168)
+++ usr/log.c (working copy)
@@ -62,15 +62,27 @@
  }
 }

+/* Definition for log_pdu buffer */
+#define BUFFER_SIZE 16
+/* Buffer line would be printed in the following format in syslog
+ * ' aa bb cc dd | ee ff gg hh | ii jj kk ll | ............... |'
+ * Means, 61 Characters if size of line is 16 chars. For N being the buffer
+ * size, the fomula would be  = (N*3 + ((N/4)+1)*2 + 3), where, the middle term
+ * is to counter printing " | " after each 4 bytes. Last '3' is to compensate
+ * for "   " printed when (i >= cnt) (and when will that happen??)
+ */
+#define LINE_SIZE (((BUFFER_SIZE)*3) + \
+ ((((int)BUFFER_SIZE/4)+1)*2) + 3)
+
 static void __dump_line(int level, unsigned char *buf, int *cp)
 {
- char line[16*3+5], *lp = line;
+ char line[LINE_SIZE], *lp = line;
  int i, cnt;

  cnt = *cp;
  if (!cnt)
  return;
- for (i = 0; i < 16; i++) {
+ for (i = 0; i < BUFFER_SIZE; i++) {
  if (i < cnt)
  lp += sprintf(lp, " %02x", buf[i]);
  else
@@ -89,7 +101,7 @@
  int cnt = (*cp)++;

  buf[cnt] = ch;
- if (cnt == 15)
+ if (cnt == (BUFFER_SIZE - 1))
  __dump_line(level, buf, cp);
 }

@@ -98,13 +110,12 @@

 void log_pdu(int level, struct PDU *pdu)
 {
- unsigned char char_buf[16];
+ unsigned char char_buf[BUFFER_SIZE];
  int char_cnt = 0;
  unsigned char *buf;
  int i;
- return;

- if (log_level <= level)
+ if (log_level < level)
  return;

  buf = (void *)&pdu->bhs;

---END---

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Iscsitarget-devel mailing list
Iscsitarget-devel@...
https://lists.sourceforge.net/lists/listinfo/iscsitarget-devel

[Arne Redlich-2]

Hi Shreyansh,

> +/* Definition for log_pdu buffer */
> +#define BUFFER_SIZE 16
> +/* Buffer line would be printed in the following format in syslog
> + * ' aa bb cc dd | ee ff gg hh | ii jj kk ll | ............... |'
> + * Means, 61 Characters if size of line is 16 chars. For N being the buffer
> + * size, the fomula would be  = (N*3 + ((N/4)+1)*2 + 3), where, the middle term
> + * is to counter printing " | " after each 4 bytes. Last '3' is to compensate
> + * for "   " printed when (i >= cnt) (and when will that happen??)
> + */
> +#define LINE_SIZE (((BUFFER_SIZE)*3) + \
> + ((((int)BUFFER_SIZE/4)+1)*2) + 3)
> +

The formatting of this is rather unusual,

#define LINE_SIZE (BUFFER_SIZE * 3 + (BUFFER_SIZE / 4 + 1) * 2 + 3)

would be preferable. I'd fix that and apply the patch if it was the only
issue, but taking a closer look at your calculation I'm afraid I fail to
understand it. IMHO, it should be:

        for (i = 0; i < BUFFER_SIZE; i++) {
                if (i < cnt)
                        lp += sprintf(lp, " %02x", buf[i]);
                else
                        lp += sprintf(lp, "   ");

=> 3 chars / per byte * BUFFER_SIZE, no matter if we're in the if or in
the else branch

                if ((i % 4) == 3)
                        lp += sprintf(lp, " |");

=> + 2 chars after each 4th byte of BUFFER_SIZE

                if (i >= cnt || !isprint(buf[i]))
                        buf[i] =  ' ';
        }

        log_debug(level, "%s %.16s |", line, buf);

=> + '\0' termination, of course, so:

#define LINE_SIZE (BUFFER_SIZE * 3 + BUFFER_SIZE / 4 * 2 + 1)

, or am I missing something?

Thanks,
Arne


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Iscsitarget-devel mailing list
Iscsitarget-devel@...
https://lists.sourceforge.net/lists/listinfo/iscsitarget-devel

[Shreyansh]

Hi Arne and List,

Thanks for your valuable time and comments. Please find my reply in line.

On Wed, Sep 3, 2008 at 12:18 AM, Arne Redlich <agr@...> wrote:

> Hi Shreyansh,
>
>> +/* Definition for log_pdu buffer */
>> +#define      BUFFER_SIZE             16
>> +/* Buffer line would be printed in the following format in syslog
>> + * ' aa bb cc dd | ee ff gg hh | ii jj kk ll | ............... |'
>> + * Means, 61 Characters if size of line is 16 chars. For N being the buffer
>> + * size, the fomula would be  = (N*3 + ((N/4)+1)*2 + 3), where, the middle term
>> + * is to counter printing " | " after each 4 bytes. Last '3' is to compensate
>> + * for "   " printed when (i >= cnt) (and when will that happen??)
>> + */
>> +#define      LINE_SIZE       (((BUFFER_SIZE)*3) + \
>> +                     ((((int)BUFFER_SIZE/4)+1)*2) + 3)
>> +
> The formatting of this is rather unusual,
>
> #define LINE_SIZE (BUFFER_SIZE * 3 + (BUFFER_SIZE / 4 + 1) * 2 + 3)

Yup, It seems absolutely correct. I really don't know what I was
thinking when I made that stupid (over) calculation error :( . I have
tried running the code with your suggested modification and it seems
to be working correctly even for larger and smaller values of
BUFFER_SIZE (than 16).

>
> would be preferable. I'd fix that and apply the patch if it was the only
> issue, but taking a closer look at your calculation I'm afraid I fail to
> understand it. IMHO, it should be:
>
>        for (i = 0; i < BUFFER_SIZE; i++) {
>                if (i < cnt)
>                        lp += sprintf(lp, " %02x", buf[i]);
>                else
>                        lp += sprintf(lp, "   ");
>
> => 3 chars / per byte * BUFFER_SIZE, no matter if we're in the if or in
> the else branch
>
>                if ((i % 4) == 3)
>                        lp += sprintf(lp, " |");
>
> => + 2 chars after each 4th byte of BUFFER_SIZE
>
>                if (i >= cnt || !isprint(buf[i]))
>                        buf[i] =  ' ';
>        }
>
>        log_debug(level, "%s %.16s |", line, buf);
>
> => + '\0' termination, of course, so:
>
> #define LINE_SIZE (BUFFER_SIZE * 3 + BUFFER_SIZE / 4 * 2 + 1)
>
> , or am I missing something?

Nope, It was me who made the mistake. Sincere apologies for wasting
your precious time.

Do you want me to re-post the complete patch for this change in a
separate email ?
I think I will as I have changed the comments above the #define
LINE_SIZE as well to fix the explanation. In case it is not required,
please ignore that.

One more thing, just for my clarification. I always thought that the
coding guidelines suggested tabs-stops between each component of a
#define. Like:
#define<tab>LINE_SIZE<tab><description of define>.

Is it OK to use <space> in place of <tab> in case the line extends
beyond 80 char column width to preserve readability?

Sincere apologies for the blunder and thanks a lot for your effort in
reviewing the patch.

----
Shreyansh

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Iscsitarget-devel mailing list
Iscsitarget-devel@...
https://lists.sourceforge.net/lists/listinfo/iscsitarget-devel

[Shreyansh]

Hi List,

As discussed in this thread, following (updated with comments) patch
fixes a buffer overrun problem in __dump_line function in log.c file.
It also removes a stray 'return' call in log_pdu function which was
restricting a PDU dump call to complete log_pdu operation.

In dump_line, the array line[] was defined as line[53] whereas sprintf
was pushing in 59 characters into it. This was leading to segfault of
ietd when debugging was enabled. Through this patch, the size of line
char array would be linked to the size of buffer defined in log_pdu
function. The calculation used is (N*3 + ((N/4)*2 + 1), where N is
the expected size of buffer used in log_pdu function.

Also, in log_pdu function, the first check for log_level against
passed parameter for log was incorrect and would have forced a return
even when the caller has set the logging level to acceptable level.

Signed-off-by: Shreyansh Jain <shrey.linux at gmail.com>

Shreyansh

---BEGIN---
Index: usr/log.c
===================================================================
--- usr/log.c (revision 1)
+++ usr/log.c (working copy)
@@ -62,15 +62,25 @@
  }
 }

+/* Definition for log_pdu buffer */
+#define BUFFER_SIZE 8
+/* Buffer line would be printed in the following format in syslog
+ * ' aa bb cc dd | ee ff gg hh | ii jj kk ll | ............... |'
+ * Means, 57 Characters if size of line is 16 chars. For N being the buffer
+ * size, the fomula would be  = (N*3 + ((N/4)*2 + 1), where, the middle term
+ * is to counter printing " |" after each 4 bytes.
+ */
+#define LINE_SIZE ((BUFFER_SIZE * 3) + (BUFFER_SIZE / 4) * 2 + 1)
+
 static void __dump_line(int level, unsigned char *buf, int *cp)
 {
- char line[16*3+5], *lp = line;
+ char line[LINE_SIZE], *lp = line;
  int i, cnt;

  cnt = *cp;
  if (!cnt)
  return;
- for (i = 0; i < 16; i++) {
+ for (i = 0; i < BUFFER_SIZE; i++) {
  if (i < cnt)
  lp += sprintf(lp, " %02x", buf[i]);
  else
@@ -89,7 +99,7 @@
  int cnt = (*cp)++;

  buf[cnt] = ch;
- if (cnt == 15)
+ if (cnt == (BUFFER_SIZE - 1))
  __dump_line(level, buf, cp);
 }

@@ -98,13 +108,12 @@

 void log_pdu(int level, struct PDU *pdu)
 {
- unsigned char char_buf[16];
+ unsigned char char_buf[BUFFER_SIZE];
  int char_cnt = 0;
  unsigned char *buf;
  int i;
- return;

- if (log_level <= level)
+ if (log_level < level)
  return;

  buf = (void *)&pdu->bhs;

---END---

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Iscsitarget-devel mailing list
Iscsitarget-devel@...
https://lists.sourceforge.net/lists/listinfo/iscsitarget-devel

[Arne Redlich-2]

Hi Shreyansh,

I merged your patch with a few modifications - rev. 169.
Below's the version I applied, hope you're ok with it.

Thanks,
Arne
---
From:  Shreyansh Jain <shrey.linux at gmail.com>
Subject: [Patch 1/1] Segfault in ietd buffer overrun in dump_line

Fix a buffer overrun problem in __dump_line function in log.c file.
It also removes a stray 'return' call in log_pdu function which was
restricting a PDU dump call to complete log_pdu operation.

Also, in log_pdu function, the first check for log_level against
passed parameter for log was incorrect and would have forced a return
even when the caller has set the logging level to acceptable level.

Signed-off-by: Shreyansh Jain <shrey.linux at gmail.com>
---
 usr/log.c |   24 +++++++++++++++++-------
 1 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/usr/log.c b/usr/log.c
index e5b178f..c90f7ef 100644
--- a/usr/log.c
+++ b/usr/log.c
@@ -62,15 +62,24 @@ void log_debug(int level, const char *fmt, ...)
  }
 }
 
+/* Definition for log_pdu buffer */
+#define BUFFER_SIZE 16
+
+/*
+ * size required for a hex dump of BUFFER_SIZE bytes (' ' + 2 chars = 3 chars
+ * per byte) with a ' |' separator each 4th byte:
+ */
+#define LINE_SIZE (BUFFER_SIZE * 3 + BUFFER_SIZE / 4 * 2 + 1)
+
 static void __dump_line(int level, unsigned char *buf, int *cp)
 {
- char line[16*3+5], *lp = line;
+ char line[LINE_SIZE], *lp = line;
  int i, cnt;
 
  cnt = *cp;
  if (!cnt)
  return;
- for (i = 0; i < 16; i++) {
+ for (i = 0; i < BUFFER_SIZE; i++) {
  if (i < cnt)
  lp += sprintf(lp, " %02x", buf[i]);
  else
@@ -80,7 +89,9 @@ static void __dump_line(int level, unsigned char *buf, int *cp)
  if (i >= cnt || !isprint(buf[i]))
  buf[i] =  ' ';
  }
- log_debug(level, "%s %.16s |", line, buf);
+
+ /* buf is not \0-terminated! */
+ log_debug(level, "%s %.*s |", line, BUFFER_SIZE, buf);
  *cp = 0;
 }
 
@@ -89,7 +100,7 @@ static void __dump_char(int level, unsigned char *buf, int *cp, int ch)
  int cnt = (*cp)++;
 
  buf[cnt] = ch;
- if (cnt == 15)
+ if (cnt == BUFFER_SIZE - 1)
  __dump_line(level, buf, cp);
 }
 
@@ -98,13 +109,12 @@ static void __dump_char(int level, unsigned char *buf, int *cp, int ch)
 
 void log_pdu(int level, struct PDU *pdu)
 {
- unsigned char char_buf[16];
+ unsigned char char_buf[BUFFER_SIZE];
  int char_cnt = 0;
  unsigned char *buf;
  int i;
- return;
 
- if (log_level <= level)
+ if (log_level < level)
  return;
 
  buf = (void *)&pdu->bhs;
--
1.5.4.3




-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Iscsitarget-devel mailing list
Iscsitarget-devel@...
https://lists.sourceforge.net/lists/listinfo/iscsitarget-devel

[rswwalker]

Arne Redlich wrote:
>
> Hi Shreyansh,
>
> I merged your patch with a few modifications - rev. 169.
> Below's the version I applied, hope you're ok with it.
>
...
> +#define LINE_SIZE (BUFFER_SIZE * 3 + BUFFER_SIZE / 4 * 2 + 1)

Arne, wouldn't it be wise to parenthesis the operations then
rely on the maintainers astute understanding of the order of
operations?

-Ross

______________________________________________________________________
This e-mail, and any attachments thereto, is intended only for use by
the addressee(s) named herein and may contain legally privileged
and/or confidential information. If you are not the intended recipient
of this e-mail, you are hereby notified that any dissemination,
distribution or copying of this e-mail, and any attachments thereto,
is strictly prohibited. If you have received this e-mail in error,
please immediately notify the sender and permanently delete the
original and any copy or printout thereof.


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Iscsitarget-devel mailing list
Iscsitarget-devel@...
https://lists.sourceforge.net/lists/listinfo/iscsitarget-devel

[Shreyansh]

Hi Arne and List,

On Fri, Sep 5, 2008 at 12:50 AM, Arne Redlich <agr@...> wrote:
> Hi Shreyansh,
>
> I merged your patch with a few modifications - rev. 169.
> Below's the version I applied, hope you're ok with it.

Thanks a lot for your help and support. I just have one doubt
regarding the change, but no objections as such.

[snip]
> +#define LINE_SIZE (BUFFER_SIZE * 3 + BUFFER_SIZE / 4 * 2 + 1)
[snip]

While writing #define, I (personally) prefer enclosing sub expansions
into brackets. I am not so very well versed with the way gcc behaves,
but with my little experience I have come to honour the age old way of
enclosing sub-expansions into brackets to avoid any problem related to
compile time and mathematical calculations. Though, I agree that the
above expression may not be so complex to create such problems.

Nevertheless, I tried compiling and all seems fine. Thanks for your
time and effort in changing the patch.

[snip]
> +
> +       /* buf is not \0-terminated! */
> +       log_debug(level, "%s %.*s |", line, BUFFER_SIZE, buf);
>        *cp = 0;
>  }
[snip]

Thanks to you, I know one more thing about variable argument list :
'%.*s' - didn't know this earlier. I always used static numbers for
string length limiters.

Regards,
Shreyansh

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Iscsitarget-devel mailing list
Iscsitarget-devel@...
https://lists.sourceforge.net/lists/listinfo/iscsitarget-devel

[Arne Redlich-2]

Hi Shreyansh,

Am Freitag, den 05.09.2008, 14:02 +0530 schrieb Shreyansh Jain:

> Hi Arne and List,
>
> On Fri, Sep 5, 2008 at 12:50 AM, Arne Redlich <agr@...> wrote:
> > Hi Shreyansh,
> >
> > I merged your patch with a few modifications - rev. 169.
> > Below's the version I applied, hope you're ok with it.
>
> Thanks a lot for your help and support. I just have one doubt
> regarding the change, but no objections as such.
>
> [snip]
> > +#define LINE_SIZE (BUFFER_SIZE * 3 + BUFFER_SIZE / 4 * 2 + 1)
> [snip]
>
> While writing #define, I (personally) prefer enclosing sub expansions
> into brackets. I am not so very well versed with the way gcc behaves,
> but with my little experience I have come to honour the age old way of
> enclosing sub-expansions into brackets to avoid any problem related to
> compile time and mathematical calculations. Though, I agree that the
> above expression may not be so complex to create such problems.

Please note that your version does *not* protect you against these bad
surprises that might be caused by, say, #define BUFFER_SIZE 10 + 6. It
merely visually reinforces C's operator precedence and associativity,
which I didn't think was that counter-intuitive that the parentheses
were required. The correct way would obviously be to use (BUFFER_SIZE)
on each occurence.

So there's only a very subjective "readability" difference between your
version and mine - obviously you and others (hello Ross :)) find the
parenthesized version easier to read, while I'm distracted by too many
levels of parentheses.

HTH,
Arne


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Iscsitarget-devel mailing list
Iscsitarget-devel@...
https://lists.sourceforge.net/lists/listinfo/iscsitarget-devel

[rswwalker]

Arne Redlich wrote:

>
> Hi Shreyansh,
>
> Am Freitag, den 05.09.2008, 14:02 +0530 schrieb Shreyansh Jain:
> > Hi Arne and List,
> >
> > On Fri, Sep 5, 2008 at 12:50 AM, Arne Redlich <agr@...> wrote:
> > > Hi Shreyansh,
> > >
> > > I merged your patch with a few modifications - rev. 169.
> > > Below's the version I applied, hope you're ok with it.
> >
> > Thanks a lot for your help and support. I just have one doubt
> > regarding the change, but no objections as such.
> >
> > [snip]
> > > +#define LINE_SIZE (BUFFER_SIZE * 3 + BUFFER_SIZE / 4 * 2 + 1)
> > [snip]
> >
> > While writing #define, I (personally) prefer enclosing sub expansions
> > into brackets. I am not so very well versed with the way gcc behaves,
> > but with my little experience I have come to honour the age old way of
> > enclosing sub-expansions into brackets to avoid any problem related to
> > compile time and mathematical calculations. Though, I agree that the
> > above expression may not be so complex to create such problems.
>
> Please note that your version does *not* protect you against these bad
> surprises that might be caused by, say, #define BUFFER_SIZE 10 + 6. It
> merely visually reinforces C's operator precedence and associativity,
> which I didn't think was that counter-intuitive that the parentheses
> were required. The correct way would obviously be to use (BUFFER_SIZE)
> on each occurence.
>
> So there's only a very subjective "readability" difference between your
> version and mine - obviously you and others (hello Ross :)) find the
> parenthesized version easier to read, while I'm distracted by too many
> levels of parentheses.

I agree that over-use of parentheses is distracting and counter
productive to code readability, but limited use of parentheses
can actually be self-documenting.

Take our little define here:

#define LINE_SIZE (BUFFER_SIZE * 3 + BUFFER_SIZE / 4 * 2 + 1)

This was determined to be:
        1) 3 chars a byte
        2) 2 chars every fourth byte
        3) 1 terminating char

Which presented as:

#define LINE_SIZE ((BUFFER_SIZE * 3) + (BUFFER_SIZE / 4 * 2) + 1)

Would help the reader to visualize the calculation IMHO, but
to each their own and this is a very subjective argument.

-Ross

______________________________________________________________________
This e-mail, and any attachments thereto, is intended only for use by
the addressee(s) named herein and may contain legally privileged
and/or confidential information. If you are not the intended recipient
of this e-mail, you are hereby notified that any dissemination,
distribution or copying of this e-mail, and any attachments thereto,
is strictly prohibited. If you have received this e-mail in error,
please immediately notify the sender and permanently delete the
original and any copy or printout thereof.


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Iscsitarget-devel mailing list
Iscsitarget-devel@...
https://lists.sourceforge.net/lists/listinfo/iscsitarget-devel

[Shreyansh]

Hi Arne, Ross and List,

>>
>> So there's only a very subjective "readability" difference between your
>> version and mine - obviously you and others (hello Ross :)) find the
>> parenthesized version easier to read, while I'm distracted by too many
>> levels of parentheses.
>
> I agree that over-use of parentheses is distracting and counter
> productive to code readability, but limited use of parentheses
> can actually be self-documenting.
>
> Take our little define here:
>
> #define LINE_SIZE (BUFFER_SIZE * 3 + BUFFER_SIZE / 4 * 2 + 1)
>
> This was determined to be:
>        1) 3 chars a byte
>        2) 2 chars every fourth byte
>        3) 1 terminating char
>
> Which presented as:
>
> #define LINE_SIZE ((BUFFER_SIZE * 3) + (BUFFER_SIZE / 4 * 2) + 1)
>
> Would help the reader to visualize the calculation IMHO, but
> to each their own and this is a very subjective argument.

I actually would like to side with Ross's view that parenthesis
increase the readability of the code - but, its also true that this
argument is quite subjective and dependent on individual view-points.
What matters here is the code correctness (primary) and all other
things are secondary.

I think Iorking 'fine' (once again, fine is a subjective argument of
what correct code is defined as).  would like to leave the decision
upon Arne (which he has already taken) as it seems to be w

Arne: Please feel free to modify the code as you (the maintainer)
deems correct. I have absolutely no issues with it (nothing on which I
can put forward an object argument).

Thanks for you help.
--
Shreyansh

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Iscsitarget-devel mailing list
Iscsitarget-devel@...
https://lists.sourceforge.net/lists/listinfo/iscsitarget-devel