On Wed, 15 Oct 2008 10:13:33 +0200 Doron Shoham <dorons at Voltaire.COM> wrote: > OK, I've tried to implement a similar checking mechanism as > open iscsi do. > > Please let me know what you think about it. Thanks a lot, The patch is different from what I thought about but it looks good. > P.S. > I'm still checking the -b option when creating a new device > because currently this option is required. > In the code we don't distinguish between the different targets. > So I'm just syncing the validation to the code itself. As I wrote in the previous mail, that's fine for now. > check for valid parameters when passing tgtadm commands > > Signed-off-by: Doron Shoham <dorons at voltaire.com> > --- > usr/tgtadm.c | 162 ++++++++++++++++++++++++++++++++++++++++++++++++--------- > 1 files changed, 136 insertions(+), 26 deletions(-) Please fix the following checkpatch errors and resend an updated patch: fujita at arbre:~/git/tgt$ ./scripts/checkpatch.pl ~/Mail/iscsi/stgt/3410 WARNING: multiple assignments should be avoided #113: FILE: usr/tgtadm.c:390: + total = cid = hostno = sid = lun = rc = 0; ERROR: do not use assignment in if condition #170: FILE: usr/tgtadm.c:514: + if ((rc = verify_mode_params(argc, argv, "LmotT"))) { WARNING: line over 80 characters #176: FILE: usr/tgtadm.c:520: + eprintf("creating a new target needs the name\n"); ERROR: do not use assignment in if condition #182: FILE: usr/tgtadm.c:526: + if ((rc = verify_mode_params(argc, argv, "Lmot"))) { ERROR: do not use assignment in if condition #190: FILE: usr/tgtadm.c:534: + if ((rc = verify_mode_params(argc, argv, "LmotI"))) { WARNING: line over 80 characters #191: FILE: usr/tgtadm.c:535: + eprintf("target mode op delete: option '-%c' is not " WARNING: line over 80 characters #196: FILE: usr/tgtadm.c:540: + eprintf("%s operation requires initiator-address\n", op==OP_BIND?"bind":"unbind"); ERROR: need spaces around that '==' (ctx:VxV) #196: FILE: usr/tgtadm.c:540: + eprintf("%s operation requires initiator-address\n", op==OP_BIND?"bind":"unbind"); ^ ERROR: do not use assignment in if condition #203: FILE: usr/tgtadm.c:545: + if ((rc = verify_mode_params(argc, argv, "Lmotnv"))) { WARNING: line over 80 characters #204: FILE: usr/tgtadm.c:546: + eprintf("target mode op delete: option '-%c' is not " WARNING: line over 80 characters #212: FILE: usr/tgtadm.c:551: + eprintf("update operation requires 'name' and 'value' options\n"); ERROR: do not use assignment in if condition #226: FILE: usr/tgtadm.c:565: + if ((rc = verify_mode_params(argc, argv, "Lmoup"))) { ERROR: do not use assignment in if condition #237: FILE: usr/tgtadm.c:576: + if ((rc = verify_mode_params(argc, argv, "Lmo"))) { ERROR: do not use assignment in if condition #244: FILE: usr/tgtadm.c:583: + if ((rc = verify_mode_params(argc, argv, "Lmou"))) { ERROR: do not use assignment in if condition #251: FILE: usr/tgtadm.c:590: + if ((rc = verify_mode_params(argc, argv, "LmotuO"))) { ERROR: do not use assignment in if condition #266: FILE: usr/tgtadm.c:605: + if ((rc = verify_mode_params(argc, argv, "Lmou"))) { WARNING: line over 80 characters #285: FILE: usr/tgtadm.c:620: + eprintf("option %d not supported in account mode\n", op); ERROR: do not use assignment in if condition #302: FILE: usr/tgtadm.c:637: + if ((rc = verify_mode_params(argc, argv, "Lmotlb"))) { WARNING: line over 80 characters #308: FILE: usr/tgtadm.c:643: + eprintf("'backing-store' option is necessary\n"); ERROR: do not use assignment in if condition #313: FILE: usr/tgtadm.c:648: + if ((rc = verify_mode_params(argc, argv, "Lmotl"))) { WARNING: line over 80 characters #320: FILE: usr/tgtadm.c:655: + eprintf("option %d not supported in logicalunit mode\n", op); Your patch has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. -- To unsubscribe from this list: send the line "unsubscribe stgt" in the body of a message to majordomo at vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html |