* [Suggestion] ISDN: isdnloop: C grammar issue, '}' miss match 'if' and 'switch' statement.
@ 2013-04-03 13:35 Chen Gang
2013-04-03 14:08 ` Michal Kubecek
0 siblings, 1 reply; 14+ messages in thread
From: Chen Gang @ 2013-04-03 13:35 UTC (permalink / raw)
To: fengguang.wu, isdn, Linus Torvalds; +Cc: David Miller, netdev
Hello Maintainers:
in drivers/isdn/isdnloop/isdnloop.c
issue description:
it is in function 'isdnloop_command'.
it seems a C grammar issue for '}' miss match 'if' and 'switch' statement
please check the line 1243, 1265, 1341.
building:
make allyesconfig, can not let it built.
in menuconfig, we (at least for me) can not let ISDN_DRV_LOOP = 'y' or 'm'.
is this module a waste module which should be deleted ?
the related commit:
commit 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2
Author: Linus Torvalds <torvalds@ppc970.osdl.org>
Date: Sat Apr 16 15:20:36 2005 -0700
please help check, thanks.
gchen.
1120 /*
1121 * Main handler for commands sent by linklevel.
1122 */
1123 static int
1124 isdnloop_command(isdn_ctrl *c, isdnloop_card *card)
1125 {
1126 ulong a;
1127 int i;
1128 char cbuf[60];
1129 isdn_ctrl cmd;
1130 isdnloop_cdef cdef;
1131
1132 switch (c->command) {
1133 case ISDN_CMD_IOCTL:
1134 memcpy(&a, c->parm.num, sizeof(ulong));
1135 switch (c->arg) {
1136 case ISDNLOOP_IOCTL_DEBUGVAR:
1137 return (ulong) card;
1138 case ISDNLOOP_IOCTL_STARTUP:
1139 if (!access_ok(VERIFY_READ, (void *) a, sizeof(isdnloop_sdef)))
1140 return -EFAULT;
1141 return (isdnloop_start(card, (isdnloop_sdef *) a));
1142 break;
1143 case ISDNLOOP_IOCTL_ADDCARD:
1144 if (copy_from_user((char *)&cdef,
1145 (char *)a,
1146 sizeof(cdef)))
1147 return -EFAULT;
1148 return (isdnloop_addcard(cdef.id1));
1149 break;
1150 case ISDNLOOP_IOCTL_LEASEDCFG:
1151 if (a) {
1152 if (!card->leased) {
1153 card->leased = 1;
1154 while (card->ptype == ISDN_PTYPE_UNKNOWN)
1155 schedule_timeout_interruptible(10);
1156 schedule_timeout_interruptible(10);
1157 sprintf(cbuf, "00;FV2ON\n01;EAZ1\n02;EAZ2\n");
1158 i = isdnloop_writecmd(cbuf, strlen(cbuf), 0, card);
1159 printk(KERN_INFO
1160 "isdnloop: (%s) Leased-line mode enabled\n",
1161 CID);
1162 cmd.command = ISDN_STAT_RUN;
1163 cmd.driver = card->myid;
1164 cmd.arg = 0;
1165 card->interface.statcallb(&cmd);
1166 }
1167 } else {
1168 if (card->leased) {
1169 card->leased = 0;
1170 sprintf(cbuf, "00;FV2OFF\n");
1171 i = isdnloop_writecmd(cbuf, strlen(cbuf), 0, card);
1172 printk(KERN_INFO
1173 "isdnloop: (%s) Leased-line mode disabled\n",
1174 CID);
1175 cmd.command = ISDN_STAT_RUN;
1176 cmd.driver = card->myid;
1177 cmd.arg = 0;
1178 card->interface.statcallb(&cmd);
1179 }
1180 }
1181 return 0;
1182 default:
1183 return -EINVAL;
1184 }
1185 break;
1186 case ISDN_CMD_DIAL:
1187 if (!(card->flags & ISDNLOOP_FLAGS_RUNNING))
1188 return -ENODEV;
1189 if (card->leased)
1190 break;
1191 if ((c->arg & 255) < ISDNLOOP_BCH) {
1192 char *p;
1193 char dial[50];
1194 char dcode[4];
1195
1196 a = c->arg;
1197 p = c->parm.setup.phone;
1198 if (*p == 's' || *p == 'S') {
1199 /* Dial for SPV */
1200 p++;
1201 strcpy(dcode, "SCA");
1202 } else
1203 /* Normal Dial */
1204 strcpy(dcode, "CAL");
1205 strcpy(dial, p);
1206 sprintf(cbuf, "%02d;D%s_R%s,%02d,%02d,%s\n", (int) (a + 1),
1207 dcode, dial, c->parm.setup.si1,
1208 c->parm.setup.si2, c->parm.setup.eazmsn);
1209 i = isdnloop_writecmd(cbuf, strlen(cbuf), 0, card);
1210 }
1211 break;
1212 case ISDN_CMD_ACCEPTD:
1213 if (!(card->flags & ISDNLOOP_FLAGS_RUNNING))
1214 return -ENODEV;
1215 if (c->arg < ISDNLOOP_BCH) {
1216 a = c->arg + 1;
1217 cbuf[0] = 0;
1218 switch (card->l2_proto[a - 1]) {
1219 case ISDN_PROTO_L2_X75I:
1220 sprintf(cbuf, "%02d;BX75\n", (int) a);
1221 break;
1222 #ifdef CONFIG_ISDN_X25
1223 case ISDN_PROTO_L2_X25DTE:
1224 sprintf(cbuf, "%02d;BX2T\n", (int) a);
1225 break;
1226 case ISDN_PROTO_L2_X25DCE:
1227 sprintf(cbuf, "%02d;BX2C\n", (int) a);
1228 break;
1229 #endif
1230 case ISDN_PROTO_L2_HDLC:
1231 sprintf(cbuf, "%02d;BTRA\n", (int) a);
1232 break;
1233 }
1234 if (strlen(cbuf))
1235 i = isdnloop_writecmd(cbuf, strlen(cbuf), 0, card);
1236 sprintf(cbuf, "%02d;DCON_R\n", (int) a);
1237 i = isdnloop_writecmd(cbuf, strlen(cbuf), 0, card);
1238 }
1239 break;
1240 case ISDN_CMD_ACCEPTB:
1241 if (!(card->flags & ISDNLOOP_FLAGS_RUNNING))
1242 return -ENODEV;
1243 if (c->arg < ISDNLOOP_BCH) {
1244 a = c->arg + 1;
1245 switch (card->l2_proto[a - 1]) {
1246 case ISDN_PROTO_L2_X75I:
1247 sprintf(cbuf, "%02d;BCON_R,BX75\n", (int) a);
1248 break;
1249 #ifdef CONFIG_ISDN_X25
1250 case ISDN_PROTO_L2_X25DTE:
1251 sprintf(cbuf, "%02d;BCON_R,BX2T\n", (int) a);
1252 break;
1253 case ISDN_PROTO_L2_X25DCE:
1254 sprintf(cbuf, "%02d;BCON_R,BX2C\n", (int) a);
1255 break;
1256 #endif
1257 case ISDN_PROTO_L2_HDLC:
1258 sprintf(cbuf, "%02d;BCON_R,BTRA\n", (int) a);
1259 break;
1260 default:
1261 sprintf(cbuf, "%02d;BCON_R\n", (int) a);
1262 }
1263 printk(KERN_DEBUG "isdnloop writecmd '%s'\n", cbuf);
1264 i = isdnloop_writecmd(cbuf, strlen(cbuf), 0, card);
1265 break;
1266 case ISDN_CMD_HANGUP:
1267 if (!(card->flags & ISDNLOOP_FLAGS_RUNNING))
1268 return -ENODEV;
1269 if (c->arg < ISDNLOOP_BCH) {
1270 a = c->arg + 1;
1271 sprintf(cbuf, "%02d;BDIS_R\n%02d;DDIS_R\n", (int) a, (int) a);
1272 i = isdnloop_writecmd(cbuf, strlen(cbuf), 0, card);
1273 }
1274 break;
1275 case ISDN_CMD_SETEAZ:
1276 if (!(card->flags & ISDNLOOP_FLAGS_RUNNING))
1277 return -ENODEV;
1278 if (card->leased)
1279 break;
1280 if (c->arg < ISDNLOOP_BCH) {
1281 a = c->arg + 1;
1282 if (card->ptype == ISDN_PTYPE_EURO) {
1283 sprintf(cbuf, "%02d;MS%s%s\n", (int) a,
1284 c->parm.num[0] ? "N" : "ALL", c->parm.num);
1285 } else
1286 sprintf(cbuf, "%02d;EAZ%s\n", (int) a,
1287 c->parm.num[0] ? c->parm.num : (u_char *) "0123456789");
1288 i = isdnloop_writecmd(cbuf, strlen(cbuf), 0, card);
1289 }
1290 break;
1291 case ISDN_CMD_CLREAZ:
1292 if (!(card->flags & ISDNLOOP_FLAGS_RUNNING))
1293 return -ENODEV;
1294 if (card->leased)
1295 break;
1296 if (c->arg < ISDNLOOP_BCH) {
1297 a = c->arg + 1;
1298 if (card->ptype == ISDN_PTYPE_EURO)
1299 sprintf(cbuf, "%02d;MSNC\n", (int) a);
1300 else
1301 sprintf(cbuf, "%02d;EAZC\n", (int) a);
1302 i = isdnloop_writecmd(cbuf, strlen(cbuf), 0, card);
1303 }
1304 break;
1305 case ISDN_CMD_SETL2:
1306 if (!(card->flags & ISDNLOOP_FLAGS_RUNNING))
1307 return -ENODEV;
1308 if ((c->arg & 255) < ISDNLOOP_BCH) {
1309 a = c->arg;
1310 switch (a >> 8) {
1311 case ISDN_PROTO_L2_X75I:
1312 sprintf(cbuf, "%02d;BX75\n", (int) (a & 255) + 1);
1313 break;
1314 #ifdef CONFIG_ISDN_X25
1315 case ISDN_PROTO_L2_X25DTE:
1316 sprintf(cbuf, "%02d;BX2T\n", (int) (a & 255) + 1);
1317 break;
1318 case ISDN_PROTO_L2_X25DCE:
1319 sprintf(cbuf, "%02d;BX2C\n", (int) (a & 255) + 1);
1320 break;
1321 #endif
1322 case ISDN_PROTO_L2_HDLC:
1323 sprintf(cbuf, "%02d;BTRA\n", (int) (a & 255) + 1);
1324 break;
1325 case ISDN_PROTO_L2_TRANS:
1326 sprintf(cbuf, "%02d;BTRA\n", (int) (a & 255) + 1);
1327 break;
1328 default:
1329 return -EINVAL;
1330 }
1331 i = isdnloop_writecmd(cbuf, strlen(cbuf), 0, card);
1332 card->l2_proto[a & 255] = (a >> 8);
1333 }
1334 break;
1335 case ISDN_CMD_SETL3:
1336 if (!(card->flags & ISDNLOOP_FLAGS_RUNNING))
1337 return -ENODEV;
1338 return 0;
1339 default:
1340 return -EINVAL;
1341 }
1342 }
1343 return 0;
1344 }
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Suggestion] ISDN: isdnloop: C grammar issue, '}' miss match 'if' and 'switch' statement.
2013-04-03 13:35 [Suggestion] ISDN: isdnloop: C grammar issue, '}' miss match 'if' and 'switch' statement Chen Gang
@ 2013-04-03 14:08 ` Michal Kubecek
2013-04-03 14:31 ` Eric Dumazet
2013-04-04 9:05 ` Chen Gang
0 siblings, 2 replies; 14+ messages in thread
From: Michal Kubecek @ 2013-04-03 14:08 UTC (permalink / raw)
To: Chen Gang
Cc: fengguang.wu, isdn, Linus Torvalds, David Miller, netdev,
Joe Perches
On Wed, Apr 03, 2013 at 09:35:55PM +0800, Chen Gang wrote:
>
> in drivers/isdn/isdnloop/isdnloop.c
>
> issue description:
> it is in function 'isdnloop_command'.
> it seems a C grammar issue for '}' miss match 'if' and 'switch' statement
> please check the line 1243, 1265, 1341.
>
> building:
> make allyesconfig, can not let it built.
> in menuconfig, we (at least for me) can not let ISDN_DRV_LOOP = 'y' or 'm'.
> is this module a waste module which should be deleted ?
>
> the related commit:
> commit 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2
> Author: Linus Torvalds <torvalds@ppc970.osdl.org>
> Date: Sat Apr 16 15:20:36 2005 -0700
As far as I can see, this rather comes from
commit 475be4d85a274d0961593db41cf85689db1d583c
Author: Joe Perches <joe@perches.com>
Date: Sun Feb 19 19:52:38 2012 -0800
isdn: whitespace coding style cleanup
> 1240 case ISDN_CMD_ACCEPTB:
> 1241 if (!(card->flags & ISDNLOOP_FLAGS_RUNNING))
> 1242 return -ENODEV;
> 1243 if (c->arg < ISDNLOOP_BCH) {
> 1244 a = c->arg + 1;
> 1245 switch (card->l2_proto[a - 1]) {
> 1246 case ISDN_PROTO_L2_X75I:
> 1247 sprintf(cbuf, "%02d;BCON_R,BX75\n", (int) a);
> 1248 break;
> 1249 #ifdef CONFIG_ISDN_X25
> 1250 case ISDN_PROTO_L2_X25DTE:
> 1251 sprintf(cbuf, "%02d;BCON_R,BX2T\n", (int) a);
> 1252 break;
> 1253 case ISDN_PROTO_L2_X25DCE:
> 1254 sprintf(cbuf, "%02d;BCON_R,BX2C\n", (int) a);
> 1255 break;
> 1256 #endif
> 1257 case ISDN_PROTO_L2_HDLC:
> 1258 sprintf(cbuf, "%02d;BCON_R,BTRA\n", (int) a);
> 1259 break;
> 1260 default:
> 1261 sprintf(cbuf, "%02d;BCON_R\n", (int) a);
> 1262 }
> 1263 printk(KERN_DEBUG "isdnloop writecmd '%s'\n", cbuf);
> 1264 i = isdnloop_writecmd(cbuf, strlen(cbuf), 0, card);
> 1265 break;
> 1266 case ISDN_CMD_HANGUP:
Michal Kubecek
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Suggestion] ISDN: isdnloop: C grammar issue, '}' miss match 'if' and 'switch' statement.
2013-04-03 14:08 ` Michal Kubecek
@ 2013-04-03 14:31 ` Eric Dumazet
2013-04-03 15:08 ` Michal Kubecek
2013-04-03 15:30 ` Linus Torvalds
2013-04-04 9:05 ` Chen Gang
1 sibling, 2 replies; 14+ messages in thread
From: Eric Dumazet @ 2013-04-03 14:31 UTC (permalink / raw)
To: Michal Kubecek
Cc: Chen Gang, fengguang.wu, isdn, Linus Torvalds, David Miller,
netdev, Joe Perches
On Wed, 2013-04-03 at 16:08 +0200, Michal Kubecek wrote:
> As far as I can see, this rather comes from
>
> commit 475be4d85a274d0961593db41cf85689db1d583c
> Author: Joe Perches <joe@perches.com>
> Date: Sun Feb 19 19:52:38 2012 -0800
>
> isdn: whitespace coding style cleanup
And what is the problem exactly ?
# make drivers/isdn/isdnloop/isdnloop.o
...
CC [M] drivers/isdn/isdnloop/isdnloop.o
No error here.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Suggestion] ISDN: isdnloop: C grammar issue, '}' miss match 'if' and 'switch' statement.
2013-04-03 14:31 ` Eric Dumazet
@ 2013-04-03 15:08 ` Michal Kubecek
2013-04-03 15:30 ` Linus Torvalds
1 sibling, 0 replies; 14+ messages in thread
From: Michal Kubecek @ 2013-04-03 15:08 UTC (permalink / raw)
To: Eric Dumazet
Cc: Chen Gang, fengguang.wu, isdn, Linus Torvalds, David Miller,
netdev, Joe Perches
On Wed, Apr 03, 2013 at 07:31:17AM -0700, Eric Dumazet wrote:
> On Wed, 2013-04-03 at 16:08 +0200, Michal Kubecek wrote:
>
> > As far as I can see, this rather comes from
> >
> > commit 475be4d85a274d0961593db41cf85689db1d583c
> > Author: Joe Perches <joe@perches.com>
> > Date: Sun Feb 19 19:52:38 2012 -0800
> >
> > isdn: whitespace coding style cleanup
>
> And what is the problem exactly ?
>
> # make drivers/isdn/isdnloop/isdnloop.o
> ...
> CC [M] drivers/isdn/isdnloop/isdnloop.o
>
> No error here.
I just checked that gcc (surprisingly - at least for me) accepts
switch(a) {
case 1:
if(...) {
do_something();
break;
case 2:
...
break;
default:
...
break;
}
}
and it seems to work the same as
switch(a) {
case 1:
if(...) {
do_something();
}
break;
case 2:
...
break;
default:
...
break;
}
However, I can't think of a reason to use the former.
Michal Kubecek
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Suggestion] ISDN: isdnloop: C grammar issue, '}' miss match 'if' and 'switch' statement.
2013-04-03 14:31 ` Eric Dumazet
2013-04-03 15:08 ` Michal Kubecek
@ 2013-04-03 15:30 ` Linus Torvalds
2013-04-04 8:30 ` Chen Gang
1 sibling, 1 reply; 14+ messages in thread
From: Linus Torvalds @ 2013-04-03 15:30 UTC (permalink / raw)
To: Eric Dumazet
Cc: Michal Kubecek, Chen Gang, Wu Fengguang, Karsten Keil,
David Miller, netdev, Joe Perches
[-- Attachment #1: Type: text/plain, Size: 1387 bytes --]
On Wed, Apr 3, 2013 at 7:31 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> And what is the problem exactly ?
The indentation does look completely broken.
It should still *work*, because case-statements don't actually care
about nesting (you can use a case statement to jump into other control
statements, the traditional example is the so-called "duff's device"),
but I agree with Chen Gang that it looks wrong.
I'm attaching a patch that would appear to fix the nesting, but I
haven't actually tested it. Also, regardless of that patch, the code
looks like complete and utter crap, because it sets the "i" variable
in many of the case statements, and then doesn't actually *use* it.
Finally, almost all of the case statements test for something like
if (c->arg < ISDNLOOP_BCH) {
but if "c->arg" is out of range, it will then just break out of the
switch statement and return 0, even though it looks like it should be
an error.
Of course, nobody sane actually cares about ISDN any more, so I think
this is all pretty academic. I think even Germany (where ISDN *used*
to be very common due to telephone monopolies and odd rules) no longer
uses it. I can't imagine that anybody else does either.
But if somebody does care, and can validate my patch (if not by
actually using it, then by at least looking at it more), feel free to
take it and take my sign-off.
Linus
[-- Attachment #2: patch.diff --]
[-- Type: application/octet-stream, Size: 4860 bytes --]
drivers/isdn/isdnloop/isdnloop.c | 140 +++++++++++++++++++--------------------
1 file changed, 70 insertions(+), 70 deletions(-)
diff --git a/drivers/isdn/isdnloop/isdnloop.c b/drivers/isdn/isdnloop/isdnloop.c
index baf2686aa8eb..992b0b78a091 100644
--- a/drivers/isdn/isdnloop/isdnloop.c
+++ b/drivers/isdn/isdnloop/isdnloop.c
@@ -1262,83 +1262,83 @@ isdnloop_command(isdn_ctrl *c, isdnloop_card *card)
}
printk(KERN_DEBUG "isdnloop writecmd '%s'\n", cbuf);
i = isdnloop_writecmd(cbuf, strlen(cbuf), 0, card);
+ }
+ break;
+ case ISDN_CMD_HANGUP:
+ if (!(card->flags & ISDNLOOP_FLAGS_RUNNING))
+ return -ENODEV;
+ if (c->arg < ISDNLOOP_BCH) {
+ a = c->arg + 1;
+ sprintf(cbuf, "%02d;BDIS_R\n%02d;DDIS_R\n", (int) a, (int) a);
+ i = isdnloop_writecmd(cbuf, strlen(cbuf), 0, card);
+ }
+ break;
+ case ISDN_CMD_SETEAZ:
+ if (!(card->flags & ISDNLOOP_FLAGS_RUNNING))
+ return -ENODEV;
+ if (card->leased)
break;
- case ISDN_CMD_HANGUP:
- if (!(card->flags & ISDNLOOP_FLAGS_RUNNING))
- return -ENODEV;
- if (c->arg < ISDNLOOP_BCH) {
- a = c->arg + 1;
- sprintf(cbuf, "%02d;BDIS_R\n%02d;DDIS_R\n", (int) a, (int) a);
- i = isdnloop_writecmd(cbuf, strlen(cbuf), 0, card);
- }
- break;
- case ISDN_CMD_SETEAZ:
- if (!(card->flags & ISDNLOOP_FLAGS_RUNNING))
- return -ENODEV;
- if (card->leased)
- break;
- if (c->arg < ISDNLOOP_BCH) {
- a = c->arg + 1;
- if (card->ptype == ISDN_PTYPE_EURO) {
- sprintf(cbuf, "%02d;MS%s%s\n", (int) a,
- c->parm.num[0] ? "N" : "ALL", c->parm.num);
- } else
- sprintf(cbuf, "%02d;EAZ%s\n", (int) a,
- c->parm.num[0] ? c->parm.num : (u_char *) "0123456789");
- i = isdnloop_writecmd(cbuf, strlen(cbuf), 0, card);
- }
+ if (c->arg < ISDNLOOP_BCH) {
+ a = c->arg + 1;
+ if (card->ptype == ISDN_PTYPE_EURO) {
+ sprintf(cbuf, "%02d;MS%s%s\n", (int) a,
+ c->parm.num[0] ? "N" : "ALL", c->parm.num);
+ } else
+ sprintf(cbuf, "%02d;EAZ%s\n", (int) a,
+ c->parm.num[0] ? c->parm.num : (u_char *) "0123456789");
+ i = isdnloop_writecmd(cbuf, strlen(cbuf), 0, card);
+ }
+ break;
+ case ISDN_CMD_CLREAZ:
+ if (!(card->flags & ISDNLOOP_FLAGS_RUNNING))
+ return -ENODEV;
+ if (card->leased)
break;
- case ISDN_CMD_CLREAZ:
- if (!(card->flags & ISDNLOOP_FLAGS_RUNNING))
- return -ENODEV;
- if (card->leased)
+ if (c->arg < ISDNLOOP_BCH) {
+ a = c->arg + 1;
+ if (card->ptype == ISDN_PTYPE_EURO)
+ sprintf(cbuf, "%02d;MSNC\n", (int) a);
+ else
+ sprintf(cbuf, "%02d;EAZC\n", (int) a);
+ i = isdnloop_writecmd(cbuf, strlen(cbuf), 0, card);
+ }
+ break;
+ case ISDN_CMD_SETL2:
+ if (!(card->flags & ISDNLOOP_FLAGS_RUNNING))
+ return -ENODEV;
+ if ((c->arg & 255) < ISDNLOOP_BCH) {
+ a = c->arg;
+ switch (a >> 8) {
+ case ISDN_PROTO_L2_X75I:
+ sprintf(cbuf, "%02d;BX75\n", (int) (a & 255) + 1);
break;
- if (c->arg < ISDNLOOP_BCH) {
- a = c->arg + 1;
- if (card->ptype == ISDN_PTYPE_EURO)
- sprintf(cbuf, "%02d;MSNC\n", (int) a);
- else
- sprintf(cbuf, "%02d;EAZC\n", (int) a);
- i = isdnloop_writecmd(cbuf, strlen(cbuf), 0, card);
- }
- break;
- case ISDN_CMD_SETL2:
- if (!(card->flags & ISDNLOOP_FLAGS_RUNNING))
- return -ENODEV;
- if ((c->arg & 255) < ISDNLOOP_BCH) {
- a = c->arg;
- switch (a >> 8) {
- case ISDN_PROTO_L2_X75I:
- sprintf(cbuf, "%02d;BX75\n", (int) (a & 255) + 1);
- break;
#ifdef CONFIG_ISDN_X25
- case ISDN_PROTO_L2_X25DTE:
- sprintf(cbuf, "%02d;BX2T\n", (int) (a & 255) + 1);
- break;
- case ISDN_PROTO_L2_X25DCE:
- sprintf(cbuf, "%02d;BX2C\n", (int) (a & 255) + 1);
- break;
+ case ISDN_PROTO_L2_X25DTE:
+ sprintf(cbuf, "%02d;BX2T\n", (int) (a & 255) + 1);
+ break;
+ case ISDN_PROTO_L2_X25DCE:
+ sprintf(cbuf, "%02d;BX2C\n", (int) (a & 255) + 1);
+ break;
#endif
- case ISDN_PROTO_L2_HDLC:
- sprintf(cbuf, "%02d;BTRA\n", (int) (a & 255) + 1);
- break;
- case ISDN_PROTO_L2_TRANS:
- sprintf(cbuf, "%02d;BTRA\n", (int) (a & 255) + 1);
- break;
- default:
- return -EINVAL;
- }
- i = isdnloop_writecmd(cbuf, strlen(cbuf), 0, card);
- card->l2_proto[a & 255] = (a >> 8);
+ case ISDN_PROTO_L2_HDLC:
+ sprintf(cbuf, "%02d;BTRA\n", (int) (a & 255) + 1);
+ break;
+ case ISDN_PROTO_L2_TRANS:
+ sprintf(cbuf, "%02d;BTRA\n", (int) (a & 255) + 1);
+ break;
+ default:
+ return -EINVAL;
}
- break;
- case ISDN_CMD_SETL3:
- if (!(card->flags & ISDNLOOP_FLAGS_RUNNING))
- return -ENODEV;
- return 0;
- default:
- return -EINVAL;
+ i = isdnloop_writecmd(cbuf, strlen(cbuf), 0, card);
+ card->l2_proto[a & 255] = (a >> 8);
}
+ break;
+ case ISDN_CMD_SETL3:
+ if (!(card->flags & ISDNLOOP_FLAGS_RUNNING))
+ return -ENODEV;
+ return 0;
+ default:
+ return -EINVAL;
}
return 0;
}
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [Suggestion] ISDN: isdnloop: C grammar issue, '}' miss match 'if' and 'switch' statement.
2013-04-03 15:30 ` Linus Torvalds
@ 2013-04-04 8:30 ` Chen Gang
2013-04-04 18:09 ` David Miller
0 siblings, 1 reply; 14+ messages in thread
From: Chen Gang @ 2013-04-04 8:30 UTC (permalink / raw)
To: Linus Torvalds
Cc: Eric Dumazet, Michal Kubecek, Wu Fengguang, Karsten Keil,
David Miller, netdev, Joe Perches
firstly, thank you very much for your details reply and your patch.
On 2013年04月03日 23:30, Linus Torvalds wrote:
> On Wed, Apr 3, 2013 at 7:31 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>>
>> And what is the problem exactly ?
>
> The indentation does look completely broken.
>
> It should still *work*, because case-statements don't actually care
> about nesting (you can use a case statement to jump into other control
> statements, the traditional example is the so-called "duff's device"),
> but I agree with Chen Gang that it looks wrong.
>
ok, thanks.
> I'm attaching a patch that would appear to fix the nesting, but I
> haven't actually tested it. Also, regardless of that patch, the code
> looks like complete and utter crap, because it sets the "i" variable
> in many of the case statements, and then doesn't actually *use* it.
> Finally, almost all of the case statements test for something like
>
> if (c->arg < ISDNLOOP_BCH) {
>
> but if "c->arg" is out of range, it will then just break out of the
> switch statement and return 0, even though it looks like it should be
> an error.
>
really, it is.
> Of course, nobody sane actually cares about ISDN any more, so I think
> this is all pretty academic. I think even Germany (where ISDN *used*
> to be very common due to telephone monopolies and odd rules) no longer
> uses it. I can't imagine that anybody else does either.
>
can we delete it ?
it will not provide contributes any more, but can waste other members'
time resources.
> But if somebody does care, and can validate my patch (if not by
> actually using it, then by at least looking at it more), feel free to
> take it and take my sign-off.
>
> Linus
>
for me, I suggest:
if we can not delete it, we'd better to apply Linus' patch.
--
Chen Gang
Asianux Corporation
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Suggestion] ISDN: isdnloop: C grammar issue, '}' miss match 'if' and 'switch' statement.
2013-04-04 8:30 ` Chen Gang
@ 2013-04-04 18:09 ` David Miller
2013-04-05 3:00 ` Joe Perches
2013-04-05 6:09 ` Chen Gang
0 siblings, 2 replies; 14+ messages in thread
From: David Miller @ 2013-04-04 18:09 UTC (permalink / raw)
To: gang.chen
Cc: torvalds, eric.dumazet, mkubecek, fengguang.wu, isdn, netdev, joe
From: Chen Gang <gang.chen@asianux.com>
Date: Thu, 04 Apr 2013 16:30:01 +0800
>> Of course, nobody sane actually cares about ISDN any more, so I think
>> this is all pretty academic. I think even Germany (where ISDN *used*
>> to be very common due to telephone monopolies and odd rules) no longer
>> uses it. I can't imagine that anybody else does either.
>>
>
> can we delete it ?
I think the point is no that we can delete it, but rather that we
should concentrate our efforts on code that more people use rather
than trying to clean up antiquated code with very few users.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Suggestion] ISDN: isdnloop: C grammar issue, '}' miss match 'if' and 'switch' statement.
2013-04-04 18:09 ` David Miller
@ 2013-04-05 3:00 ` Joe Perches
2013-04-05 6:13 ` Chen Gang
2013-04-05 20:37 ` Dan Williams
2013-04-05 6:09 ` Chen Gang
1 sibling, 2 replies; 14+ messages in thread
From: Joe Perches @ 2013-04-05 3:00 UTC (permalink / raw)
To: David Miller
Cc: gang.chen, torvalds, eric.dumazet, mkubecek, fengguang.wu, isdn,
netdev
On Thu, 2013-04-04 at 14:09 -0400, David Miller wrote:
> From: Chen Gang <gang.chen@asianux.com>
> Date: Thu, 04 Apr 2013 16:30:01 +0800
> >> Of course, nobody sane actually cares about ISDN any more, so I think
> >> this is all pretty academic. I think even Germany (where ISDN *used*
> >> to be very common due to telephone monopolies and odd rules) no longer
> >> uses it. I can't imagine that anybody else does either.
> > can we delete it ?
> I think the point is no that we can delete it, but rather that we
> should concentrate our efforts on code that more people use rather
> than trying to clean up antiquated code with very few users.
Very sensible.
I was going to see about moving drivers/isdn/hardware/eicon
to staging but it seems the hardware is still for sale.
http://www.dialogic.com/en/products/media/diva/diva-bri-2.aspx
Anyone have suggestions on other obsolete isdn drivers that
could be moved to staging for awhile before deletion?
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Suggestion] ISDN: isdnloop: C grammar issue, '}' miss match 'if' and 'switch' statement.
2013-04-05 3:00 ` Joe Perches
@ 2013-04-05 6:13 ` Chen Gang
2013-04-05 20:37 ` Dan Williams
1 sibling, 0 replies; 14+ messages in thread
From: Chen Gang @ 2013-04-05 6:13 UTC (permalink / raw)
To: Joe Perches
Cc: David Miller, torvalds, eric.dumazet, mkubecek, fengguang.wu,
isdn, netdev
On 2013年04月05日 11:00, Joe Perches wrote:
> On Thu, 2013-04-04 at 14:09 -0400, David Miller wrote:
>> > From: Chen Gang <gang.chen@asianux.com>
>> > Date: Thu, 04 Apr 2013 16:30:01 +0800
>>>> > >> Of course, nobody sane actually cares about ISDN any more, so I think
>>>> > >> this is all pretty academic. I think even Germany (where ISDN *used*
>>>> > >> to be very common due to telephone monopolies and odd rules) no longer
>>>> > >> uses it. I can't imagine that anybody else does either.
>>> > > can we delete it ?
>> > I think the point is no that we can delete it, but rather that we
>> > should concentrate our efforts on code that more people use rather
>> > than trying to clean up antiquated code with very few users.
> Very sensible.
>
> I was going to see about moving drivers/isdn/hardware/eicon
> to staging but it seems the hardware is still for sale.
>
> http://www.dialogic.com/en/products/media/diva/diva-bri-2.aspx
>
> Anyone have suggestions on other obsolete isdn drivers that
> could be moved to staging for awhile before deletion?
>
>
>
thank you for your information
--
Chen Gang
Asianux Corporation
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Suggestion] ISDN: isdnloop: C grammar issue, '}' miss match 'if' and 'switch' statement.
2013-04-05 3:00 ` Joe Perches
2013-04-05 6:13 ` Chen Gang
@ 2013-04-05 20:37 ` Dan Williams
2013-04-06 4:56 ` Chen Gang
1 sibling, 1 reply; 14+ messages in thread
From: Dan Williams @ 2013-04-05 20:37 UTC (permalink / raw)
To: Joe Perches
Cc: David Miller, gang.chen, torvalds, eric.dumazet, mkubecek,
fengguang.wu, isdn, netdev
On Thu, 2013-04-04 at 20:00 -0700, Joe Perches wrote:
> On Thu, 2013-04-04 at 14:09 -0400, David Miller wrote:
> > From: Chen Gang <gang.chen@asianux.com>
> > Date: Thu, 04 Apr 2013 16:30:01 +0800
> > >> Of course, nobody sane actually cares about ISDN any more, so I think
> > >> this is all pretty academic. I think even Germany (where ISDN *used*
> > >> to be very common due to telephone monopolies and odd rules) no longer
> > >> uses it. I can't imagine that anybody else does either.
> > > can we delete it ?
> > I think the point is no that we can delete it, but rather that we
> > should concentrate our efforts on code that more people use rather
> > than trying to clean up antiquated code with very few users.
>
> Very sensible.
>
> I was going to see about moving drivers/isdn/hardware/eicon
> to staging but it seems the hardware is still for sale.
>
> http://www.dialogic.com/en/products/media/diva/diva-bri-2.aspx
>
> Anyone have suggestions on other obsolete isdn drivers that
> could be moved to staging for awhile before deletion?
Anything PCMCIA or ISA? There are a number of them:
CONFIG_ISDN_DRV_AVMB1_B1ISA
CONFIG_ISDN_DRV_AVMB1_B1PCMCIA
CONFIG_ISDN_DRV_AVMB1_AVM_CS
CONFIG_ISDN_DRV_AVMB1_T1ISA
CONFIG_HISAX_SEDLBAUER_CS
CONFIG_HISAX_ELSA_CS
CONFIG_HISAX_AVM_A1_CS
CONFIG_HISAX_TELES_CS
CONFIG_ISDN_DRV_ACT2000
That's all I can find in a quick look.
Dan
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Suggestion] ISDN: isdnloop: C grammar issue, '}' miss match 'if' and 'switch' statement.
2013-04-05 20:37 ` Dan Williams
@ 2013-04-06 4:56 ` Chen Gang
0 siblings, 0 replies; 14+ messages in thread
From: Chen Gang @ 2013-04-06 4:56 UTC (permalink / raw)
To: Dan Williams
Cc: Joe Perches, David Miller, torvalds, eric.dumazet, mkubecek,
fengguang.wu, isdn, netdev
On 2013年04月06日 04:37, Dan Williams wrote:
> On Thu, 2013-04-04 at 20:00 -0700, Joe Perches wrote:
>> > On Thu, 2013-04-04 at 14:09 -0400, David Miller wrote:
>>> > > From: Chen Gang <gang.chen@asianux.com>
>>> > > Date: Thu, 04 Apr 2013 16:30:01 +0800
>>>>> > > >> Of course, nobody sane actually cares about ISDN any more, so I think
>>>>> > > >> this is all pretty academic. I think even Germany (where ISDN *used*
>>>>> > > >> to be very common due to telephone monopolies and odd rules) no longer
>>>>> > > >> uses it. I can't imagine that anybody else does either.
>>>> > > > can we delete it ?
>>> > > I think the point is no that we can delete it, but rather that we
>>> > > should concentrate our efforts on code that more people use rather
>>> > > than trying to clean up antiquated code with very few users.
>> >
>> > Very sensible.
>> >
>> > I was going to see about moving drivers/isdn/hardware/eicon
>> > to staging but it seems the hardware is still for sale.
>> >
>> > http://www.dialogic.com/en/products/media/diva/diva-bri-2.aspx
>> >
>> > Anyone have suggestions on other obsolete isdn drivers that
>> > could be moved to staging for awhile before deletion?
> Anything PCMCIA or ISA? There are a number of them:
>
> CONFIG_ISDN_DRV_AVMB1_B1ISA
> CONFIG_ISDN_DRV_AVMB1_B1PCMCIA
> CONFIG_ISDN_DRV_AVMB1_AVM_CS
> CONFIG_ISDN_DRV_AVMB1_T1ISA
> CONFIG_HISAX_SEDLBAUER_CS
> CONFIG_HISAX_ELSA_CS
> CONFIG_HISAX_AVM_A1_CS
> CONFIG_HISAX_TELES_CS
> CONFIG_ISDN_DRV_ACT2000
>
> That's all I can find in a quick look.
thanks. it seems we really can not delete ISDN.
:-)
--
Chen Gang
Asianux Corporation
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Suggestion] ISDN: isdnloop: C grammar issue, '}' miss match 'if' and 'switch' statement.
2013-04-04 18:09 ` David Miller
2013-04-05 3:00 ` Joe Perches
@ 2013-04-05 6:09 ` Chen Gang
1 sibling, 0 replies; 14+ messages in thread
From: Chen Gang @ 2013-04-05 6:09 UTC (permalink / raw)
To: David Miller
Cc: torvalds, eric.dumazet, mkubecek, fengguang.wu, isdn, netdev, joe
On 2013年04月05日 02:09, David Miller wrote:
> From: Chen Gang <gang.chen@asianux.com>
> Date: Thu, 04 Apr 2013 16:30:01 +0800
>
>>> >> Of course, nobody sane actually cares about ISDN any more, so I think
>>> >> this is all pretty academic. I think even Germany (where ISDN *used*
>>> >> to be very common due to telephone monopolies and odd rules) no longer
>>> >> uses it. I can't imagine that anybody else does either.
>>> >>
>> >
>> > can we delete it ?
> I think the point is no that we can delete it, but rather that we
> should concentrate our efforts on code that more people use rather
> than trying to clean up antiquated code with very few users.
>
>
ok, we should respect the opinions of the related maintainers.
although for me:
I still suggest to apply the Linus' patch, if we do not delete ISDN.
next, I will not be still focus on ISDN.
BTW:
the reasons why I am interested in ISDN are:
a: I am trying to improve finding issues ability, by reading code.
b: quite a few of ISDN codes seem quite surprising or strange.
c: when I am reading, I make several mistakes and misunderstanding.
the above 3 are the whole reasons why I am interested in ISDN.
thanks.
:-)
--
Chen Gang
Asianux Corporation
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Suggestion] ISDN: isdnloop: C grammar issue, '}' miss match 'if' and 'switch' statement.
2013-04-03 14:08 ` Michal Kubecek
2013-04-03 14:31 ` Eric Dumazet
@ 2013-04-04 9:05 ` Chen Gang
2013-04-04 14:42 ` Joe Perches
1 sibling, 1 reply; 14+ messages in thread
From: Chen Gang @ 2013-04-04 9:05 UTC (permalink / raw)
To: Michal Kubecek
Cc: fengguang.wu, isdn, Linus Torvalds, David Miller, netdev,
Joe Perches
On 2013年04月03日 22:08, Michal Kubecek wrote:
> On Wed, Apr 03, 2013 at 09:35:55PM +0800, Chen Gang wrote:
>> >
>> > in drivers/isdn/isdnloop/isdnloop.c
>> >
>> > issue description:
>> > it is in function 'isdnloop_command'.
>> > it seems a C grammar issue for '}' miss match 'if' and 'switch' statement
>> > please check the line 1243, 1265, 1341.
>> >
>> > building:
>> > make allyesconfig, can not let it built.
>> > in menuconfig, we (at least for me) can not let ISDN_DRV_LOOP = 'y' or 'm'.
>> > is this module a waste module which should be deleted ?
>> >
>> > the related commit:
>> > commit 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2
>> > Author: Linus Torvalds <torvalds@ppc970.osdl.org>
>> > Date: Sat Apr 16 15:20:36 2005 -0700
> As far as I can see, this rather comes from
>
> commit 475be4d85a274d0961593db41cf85689db1d583c
> Author: Joe Perches <joe@perches.com>
> Date: Sun Feb 19 19:52:38 2012 -0800
Joe Perches only beautified the code, not change the contents.
please check thanks.
--
Chen Gang
Asianux Corporation
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Suggestion] ISDN: isdnloop: C grammar issue, '}' miss match 'if' and 'switch' statement.
2013-04-04 9:05 ` Chen Gang
@ 2013-04-04 14:42 ` Joe Perches
0 siblings, 0 replies; 14+ messages in thread
From: Joe Perches @ 2013-04-04 14:42 UTC (permalink / raw)
To: Chen Gang
Cc: Michal Kubecek, fengguang.wu, isdn, Linus Torvalds, David Miller,
netdev
On Thu, 2013-04-04 at 17:05 +0800, Chen Gang wrote:
> > As far as I can see, this rather comes from
> > commit 475be4d85a274d0961593db41cf85689db1d583c
[]
> Joe Perches only beautified the code, not change the contents.
I modified alll those files using emacs
c-indent-line-or-region
which does a decent job in most cases
but emacs is easily confused.
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2013-04-06 4:57 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-03 13:35 [Suggestion] ISDN: isdnloop: C grammar issue, '}' miss match 'if' and 'switch' statement Chen Gang
2013-04-03 14:08 ` Michal Kubecek
2013-04-03 14:31 ` Eric Dumazet
2013-04-03 15:08 ` Michal Kubecek
2013-04-03 15:30 ` Linus Torvalds
2013-04-04 8:30 ` Chen Gang
2013-04-04 18:09 ` David Miller
2013-04-05 3:00 ` Joe Perches
2013-04-05 6:13 ` Chen Gang
2013-04-05 20:37 ` Dan Williams
2013-04-06 4:56 ` Chen Gang
2013-04-05 6:09 ` Chen Gang
2013-04-04 9:05 ` Chen Gang
2013-04-04 14:42 ` Joe Perches
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).