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
