Re: [PATCH] check for valid parameters when passing tgtadm commands

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 15 Oct 2008 10:13:33 +0200
Doron Shoham <dorons@xxxxxxxxxxxx> 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@xxxxxxxxxxxx>
> ---
>  usr/tgtadm.c |  162 ++++++++++++++++++++++++++++++++++++++++++++++++---------
>  1 files changed, 136 insertions(+), 26 deletions(-)

Please fix the following checkpatch errors and resend an updated
patch:

fujita@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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux SCSI]     [Linux RAID]     [Linux Clusters]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]

  Powered by Linux