Fixed consistent spacing around '*'.
Signed-off-by: Alice Ferrazzi <[email protected]>
---
drivers/staging/comedi/drivers/serial2002.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/comedi/drivers/serial2002.c b/drivers/staging/comedi/drivers/serial2002.c
index e6177b4..858d364 100644
--- a/drivers/staging/comedi/drivers/serial2002.c
+++ b/drivers/staging/comedi/drivers/serial2002.c
@@ -604,7 +604,7 @@ static int serial_2002_open(struct comedi_device *dev)
c[j].max;
range_table_list[chan] =
(const struct
- comedi_lrange *)
+ comedi_lrange*)
&range[j];
}
maxdata_list[chan] =
--
1.7.10.4
On Wed, Mar 20, 2013 at 09:26:51PM +0900, Alice Ferrazzi wrote:
> Fixed consistent spacing around '*'.
>
The original was correct, actually.
> Signed-off-by: Alice Ferrazzi <[email protected]>
> ---
> drivers/staging/comedi/drivers/serial2002.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/comedi/drivers/serial2002.c b/drivers/staging/comedi/drivers/serial2002.c
> index e6177b4..858d364 100644
> --- a/drivers/staging/comedi/drivers/serial2002.c
> +++ b/drivers/staging/comedi/drivers/serial2002.c
> @@ -604,7 +604,7 @@ static int serial_2002_open(struct comedi_device *dev)
> c[j].max;
> range_table_list[chan] =
> (const struct
> - comedi_lrange *)
> + comedi_lrange*)
> &range[j];
The original code here needs to broken up into functions so it isn't
squashed up against the 80 character limit.
For casts the spacing should look like:
foo = (struct my_struct *)ptr;
There is a space after "my_struct" but no space after the closing
parenthesis. Use that to remind yourself that casting is a high
precedence operation.
For declaring pointers the spacing is:
struct my_struct *ptr;
For multiplication the spacing is:
foo = x * y;
Or multplication with a dereference it would be:
foo = x * *ptr;
regards,
dan carpenter
On Wed, Mar 20, 2013 at 03:47:53PM +0300, Dan Carpenter wrote:
> The original code here needs to broken up into functions so it isn't
> squashed up against the 80 character limit.
I'd say what needs to be done to the original code...
Observation 1:
if (foo) {
A /* two lines */
} else {
B /* huge pile of shite */
}
return result;
is equivalent to
if (foo) {
A
return result;
}
B
return result;
Observation 2:
while (1) {
A /* a couple of lines */
if (foo) {
break;
} else {
B /* huge pile of shite */
}
}
is equivalent to
while (1) {
A
if (foo)
break;
B
}
Observation 3:
while (1) {
A /* moderate pile of shite, assigning foo */
if (foo) {
B /* huge pile of shite */
}
}
is equivalent to
while (1) {
A
if (!foo)
continue;
B
}
Observation 4: functions are there for purpose. When you have two identical
piles of garbage (avert your eyes, or risk taking another look at your dinner)
such as
int unit, sign, min;
unit =
(data.value >> 10) &
0x7;
sign =
(data.value >> 13) &
0x1;
min =
(data.value >> 14) &
0xfffff;
switch (unit) {
case 0:{
min =
min
*
1000000;
}
break;
case 1:{
min =
min
*
1000;
}
break;
case 2:{
min =
min
* 1;
}
break;
}
if (sign)
min = -min;
you just might consider turning that pile of excrements into a helper
function. Incidentally, min = min * 1 is somewhat, er, pointless...
Observation 5:
for (i = 0; i <= 4; i++) {
{
switch (i) {
case 0: c = non_NULL_1; ... break;
case 1: c = non_NULL_2; ... break;
case 2: c = non_NULL_3; ... break;
case 3: c = non_NULL_4; ... break;
case 4: c = non_NULL_5; ... break;
default: c = NULL; break;
}
if (c) {
pile_of_shite
}
}
might, perhaps, be taking defensive programming a bit too far...
Observation 6:
the Vogon whose brain has produced that code up had been brought up on Pascal,
Ada or something worse, and had been badly traumatized by semantics of switch
and break.
switch (foo) {
case 0: {
bar = baz;
} break;
case 1: {
.....
}
is not quite conventional for C.
Observation 7: down, not across...
On Wed, 2013-03-20 at 15:29 +0000, Al Viro wrote:
> On Wed, Mar 20, 2013 at 03:47:53PM +0300, Dan Carpenter wrote:
>
> > The original code here needs to broken up into functions so it isn't
> > squashed up against the 80 character limit.
>
> I'd say what needs to be done to the original code...
All good things, thanks for taking the time to write it Al.
Not that I'm ever touching comedi code, but Hartley is, so
I'm adding him to cc just in case he didn't see it...
> Observation 1:
> if (foo) {
> A /* two lines */
> } else {
> B /* huge pile of shite */
> }
> return result;
>
> is equivalent to
>
> if (foo) {
> A
> return result;
> }
> B
> return result;
>
> Observation 2:
> while (1) {
> A /* a couple of lines */
> if (foo) {
> break;
> } else {
> B /* huge pile of shite */
> }
> }
> is equivalent to
> while (1) {
> A
> if (foo)
> break;
> B
> }
>
> Observation 3:
> while (1) {
> A /* moderate pile of shite, assigning foo */
> if (foo) {
> B /* huge pile of shite */
> }
> }
> is equivalent to
> while (1) {
> A
> if (!foo)
> continue;
> B
> }
>
> Observation 4: functions are there for purpose. When you have two identical
> piles of garbage (avert your eyes, or risk taking another look at your dinner)
> such as
> int unit, sign, min;
> unit =
> (data.value >> 10) &
> 0x7;
> sign =
> (data.value >> 13) &
> 0x1;
> min =
> (data.value >> 14) &
> 0xfffff;
>
> switch (unit) {
> case 0:{
> min =
> min
> *
> 1000000;
> }
> break;
> case 1:{
> min =
> min
> *
> 1000;
> }
> break;
> case 2:{
> min =
> min
> * 1;
> }
> break;
> }
> if (sign)
> min = -min;
>
> you just might consider turning that pile of excrements into a helper
> function. Incidentally, min = min * 1 is somewhat, er, pointless...
>
> Observation 5:
> for (i = 0; i <= 4; i++) {
> {
> switch (i) {
> case 0: c = non_NULL_1; ... break;
> case 1: c = non_NULL_2; ... break;
> case 2: c = non_NULL_3; ... break;
> case 3: c = non_NULL_4; ... break;
> case 4: c = non_NULL_5; ... break;
> default: c = NULL; break;
> }
> if (c) {
> pile_of_shite
> }
> }
> might, perhaps, be taking defensive programming a bit too far...
>
> Observation 6:
> the Vogon whose brain has produced that code up had been brought up on Pascal,
> Ada or something worse, and had been badly traumatized by semantics of switch
> and break.
> switch (foo) {
> case 0: {
> bar = baz;
> } break;
> case 1: {
> .....
> }
> is not quite conventional for C.
>
> Observation 7: down, not across...
On Wednesday, March 20, 2013 9:04 AM, Joe Perches wrote:
> On Wed, 2013-03-20 at 15:29 +0000, Al Viro wrote:
>> On Wed, Mar 20, 2013 at 03:47:53PM +0300, Dan Carpenter wrote:
>>
>>> The original code here needs to broken up into functions so it isn't
>>> squashed up against the 80 character limit.
>>
>> I'd say what needs to be done to the original code...
>
> All good things, thanks for taking the time to write it Al.
> Not that I'm ever touching comedi code, but Hartley is, so
> I'm adding him to cc just in case he didn't see it...
I saw it.
I haven't touched this driver yet because I haven't worked out
a clean way to fix the sparse warnings in it.
I'll get to it eventually. :-)
Regards,
Hartley
Add Al's comments in from https://lkml.org/lkml/2013/3/20/345
Signed-off-by: Joe Perches <[email protected]>
---
diff --git a/Documentation/CodingStyle b/Documentation/CodingStyle
index e00b8f0..c4ba183 100644
--- a/Documentation/CodingStyle
+++ b/Documentation/CodingStyle
@@ -836,6 +836,141 @@ next instruction in the assembly output:
: /* outputs */ : /* inputs */ : /* clobbers */);
+ Chapter 20:
+
+Tips to avoid overly tab indented code.
+
+Tip 1:
+ if (foo) {
+ A /* two lines */
+ } else {
+ B /* huge pile of shite */
+ }
+ return result;
+
+is equivalent to
+
+ if (foo) {
+ A
+ return result;
+ }
+ B
+ return result;
+
+Tip 2:
+ while (1) {
+ A /* a couple of lines */
+ if (foo) {
+ break;
+ } else {
+ B /* huge pile of shite */
+ }
+ }
+
+is equivalent to
+
+ while (1) {
+ A
+ if (foo)
+ break;
+ B
+ }
+
+Tip 3:
+ while (1) {
+ A /* moderate pile of shite, assigning foo */
+ if (foo) {
+ B /* huge pile of shite */
+ }
+ }
+
+is equivalent to
+
+ while (1) {
+ A
+ if (!foo)
+ continue;
+ B
+ }
+
+Tip 4:
+
+functions are there for purpose. When you have two identical piles of
+garbage (avert your eyes, or risk taking another look at your dinner)
+such as
+ int unit, sign, min;
+ unit =
+ (data.value >> 10) &
+ 0x7;
+ sign =
+ (data.value >> 13) &
+ 0x1;
+ min =
+ (data.value >> 14) &
+ 0xfffff;
+
+ switch (unit) {
+ case 0:{
+ min =
+ min
+ *
+ 1000000;
+ }
+ break;
+ case 1:{
+ min =
+ min
+ *
+ 1000;
+ }
+ break;
+ case 2:{
+ min =
+ min
+ * 1;
+ }
+ break;
+ }
+ if (sign)
+ min = -min;
+
+you just might consider turning that pile of excrements into a helper
+function. Incidentally, min = min * 1 is somewhat, er, pointless...
+
+Tip 5:
+ for (i = 0; i <= 4; i++) {
+ {
+ switch (i) {
+ case 0: c = non_NULL_1; ... break;
+ case 1: c = non_NULL_2; ... break;
+ case 2: c = non_NULL_3; ... break;
+ case 3: c = non_NULL_4; ... break;
+ case 4: c = non_NULL_5; ... break;
+ default: c = NULL; break;
+ }
+ if (c) {
+ pile_of_shite
+ }
+ }
+might, perhaps, be taking defensive programming a bit too far...
+
+Tip 6:
+The Vogon whose brain has produced that code up had been brought up on Pascal,
+Ada or something worse, and had been badly traumatized by semantics of switch
+and break.
+ switch (foo) {
+ case 0: {
+ bar = baz;
+ } break;
+ case 1: {
+ .....
+ }
+ is not quite conventional for C.
+
+Tip 7:
+Code flow is down, not across...
+
+
Appendix I: References
On Wed, Mar 20, 2013 at 09:26:54AM -0700, Joe Perches wrote:
> Add Al's comments in from https://lkml.org/lkml/2013/3/20/345
> +Tip 7:
> +Code flow is down, not across...
That one is a lovely improvement on what I actually said. google
"down, not across" for context...
On Wed, 2013-03-20 at 16:52 +0000, Al Viro wrote:
> On Wed, Mar 20, 2013 at 09:26:54AM -0700, Joe Perches wrote:
> > Add Al's comments in from https://lkml.org/lkml/2013/3/20/345
>
> > +Tip 7:
> > +Code flow is down, not across...
>
> That one is a lovely improvement on what I actually said. google
> "down, not across" for context...
Heh. Context is everything.
I shoulda known better, given the source ;)
cheers, Joe