Skip to content
This repository was archived by the owner on May 3, 2024. It is now read-only.

Conversation

@gaurkrgaurav
Copy link
Contributor

@gaurkrgaurav gaurkrgaurav commented Aug 5, 2022

This patch fixes some of the Codacy warnings.
warning fixed: "Double quote to prevent globing and words splitting and issues like $(...)".

Co-authored-by: Venkatesh Balagani [email protected]
Signed-off-by: Gaurav Gaur [email protected]

Problem Statement

  • We see 1688 occurrences of the pattern, "Double quote to prevent globing and word splitting".
  • And issues like Use $(...) notation instead of legacy backticked ....

Design

  • We are putting the variable references in double-quotes.
  • Also fixed issues like Use $(...) notation instead of legacy backticked ....

Coding

Checklist for Author

  • Coding conventions are followed and code is consistent

Testing

Checklist for Author

  • Unit and System Tests are added
  • Test Cases cover Happy Path, Non-Happy Path and Scalability
  • Testing was performed with RPM

Impact Analysis

Checklist for Author/Reviewer/GateKeeper

  • Interface change (if any) are documented
  • Side effects on other features (deployment/upgrade)
  • Dependencies on other component(s)

Review Checklist

Checklist for Author

  • JIRA number/GitHub Issue added to PR
  • PR is self reviewed
  • Jira and state/status is updated and JIRA is updated with PR link
  • Check if the description is clear and explained

Documentation

Checklist for Author

  • Changes done to WIKI / Confluence page / Quick Start Guide

warning fixed : "Double quote to prevent globing and words splitting".

Co-authored-by: Venkatesh Balagani <[email protected]>
Signed-off-by: Gaurav Gaur <[email protected]>
@gaurkrgaurav
Copy link
Contributor Author

Ran Lnet sanity & Test passed:
[root@ssc-vm-g4-rhev4-1908 x86_64]# /opt/seagate/cortx/motr/sanity/cortx_lnet_sanity.sh

This script [/opt/seagate/cortx/motr/sanity/cortx_lnet_sanity.sh] will take the following actions
1. Verify that Lustre client packages [kmod-lustre-client lustre-client]
are installed.
2. Verify that the file [/etc/modprobe.d/lnet.conf] exists.
3. Parse the LNET configuration and validate the nids by executing
[sudo lctl list_nids].
a. This script will not load the Lustre modules, or verify the loaded
status.
b. The script expects the Lustre modules are loaded.
c. Error is identified by failing [sudo lctl list_nids] command.
4. On success, it will return Zero.

[MSG] Collecting RPM list

[MSG] RPM list collected

[MSG] LNET component [kmod-lustre-client] is installed.

[MSG] LNET component [lustre-client] is installed.

[MSG] LNET conf file found here /etc/modprobe.d/lnet.conf

[MSG] LNET conf file found here /etc/modprobe.d/lnet.conf is not empty.

[MSG] Found configured device: [eth1]

[MSG] Configured device: [eth1] has address [192.168.53.119].

[MSG] IP_ADDR [192.168.53.119] is a valid and reachable IP address

[MSG] NID [192.168.53.119@tcp] found device [eth1] IP [192.168.53.119]

[MSG] Terminating with RESULT [0].

@rkothiya
Copy link
Contributor

rkothiya commented Aug 5, 2022

Jenkins CI Result : Motr#1567

Motr Test Summary

Test ResultCountInfo
❌Failed2
📁

04motr-single-node/49motr-rpc-cancel
01motr-single-node/00userspace-tests

🏁Skipped31
📁

01motr-single-node/28sys-kvs
01motr-single-node/35m0singlenode
01motr-single-node/04initscripts
02motr-single-node/51kem
02motr-single-node/20rpc-session-cancel
02motr-single-node/10pver-assign
02motr-single-node/21fsync-single-node
02motr-single-node/13dgmode-io
02motr-single-node/14poolmach
02motr-single-node/11m0t1fs
02motr-single-node/26motr-user-kernel-tests
02motr-single-node/08spiel
03motr-single-node/06conf
03motr-single-node/36spare-reservation
04motr-single-node/34sns-repair-1n-1f
04motr-single-node/08spiel-sns-repair-quiesce
04motr-single-node/28sys-kvs-kernel
04motr-single-node/11m0t1fs-rconfc-fail
04motr-single-node/08spiel-sns-repair
04motr-single-node/19sns-repair-abort
04motr-single-node/22sns-repair-ios-fail
05motr-single-node/18sns-repair-quiesce
05motr-single-node/12fwait
05motr-single-node/16sns-repair-multi
05motr-single-node/07mount-fail
05motr-single-node/15sns-repair-single
05motr-single-node/23sns-abort-quiesce
05motr-single-node/17sns-repair-concurrent-io
05motr-single-node/07mount
05motr-single-node/07mount-multiple
05motr-single-node/12fsync

✔️Passed42
📁

01motr-single-node/43m0crate
01motr-single-node/05confgen
01motr-single-node/06hagen
01motr-single-node/52motr-singlenode-sanity
01motr-single-node/01net
01motr-single-node/01kernel-tests
01motr-single-node/03console
01motr-single-node/37protocol
01motr-single-node/02rpcping
02motr-single-node/07m0d-fatal
02motr-single-node/67fdmi-plugin-multi-filters
02motr-single-node/53clusterusage-alert
02motr-single-node/41motr-conf-update
03motr-single-node/61sns-repair-motr-1n-1f
03motr-single-node/72spiel-sns-motr-repair-quiesce
03motr-single-node/08spiel-multi-confd
03motr-single-node/69sns-repair-motr-quiesce
03motr-single-node/62sns-repair-motr-mf
03motr-single-node/70sns-failure-after-repair-quiesce
03motr-single-node/63sns-repair-motr-1k-1f
03motr-single-node/60sns-repair-motr-1f
03motr-single-node/66sns-repair-motr-abort-quiesce
03motr-single-node/24motr-dix-repair-lookup-insert-spiel
03motr-single-node/68sns-repair-motr-shutdown
03motr-single-node/64sns-repair-motr-ios-fail
03motr-single-node/71spiel-sns-motr-repair
03motr-single-node/24motr-dix-repair-lookup-insert-m0repair
03motr-single-node/04sss
03motr-single-node/65sns-repair-motr-abort
04motr-single-node/48motr-raid0-io
04motr-single-node/25m0kv
04motr-single-node/44motr-rm-lock-cc-io
04motr-single-node/45motr-rmw
05motr-single-node/23dix-repair-m0repair
05motr-single-node/43motr-sync-replication
05motr-single-node/42motr-utils
05motr-single-node/45motr-sns-repair-N-1
05motr-single-node/40motr-dgmode
05motr-single-node/23dix-repair-quiesce-m0repair
05motr-single-node/23spiel-dix-repair-quiesce
05motr-single-node/44motr-sns-repair
05motr-single-node/23spiel-dix-repair

Total75🔗

CppCheck Summary

   Cppcheck: No new warnings found 👍

@gaurkrgaurav
Copy link
Contributor Author

@upendrapatwardhan @kanchan-chaudhari Can you please review this PR?

Copy link
Contributor

@kanchan-chaudhari kanchan-chaudhari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks Good.

@hessio hessio added codacy Status: Waiting to be Reviewed PR is waiting for reviewers to review the PR labels Aug 9, 2022
verify_lnet_conf_data()
{
local RESULT=$ERR_LNET_BAD_CONF_DATA
local RESULT=${ERR_LNET_BAD_CONF_DATA}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$ERR_LNET_BAD_CONF_DATA and ${ERR_LNET_BAD_CONF_DATA} are two different ways to reference a variable.

What is the reason to change this?
If you want to change this, we need to change all places. But this patch only changes some of them.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And the commit message is to remove the "double quoting" problem.
this kind of error/warning is not mentioned in the commit log.
Is this a real warning or error?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$ERR_LNET_BAD_CONF_DATA and ${ERR_LNET_BAD_CONF_DATA} are two different ways to reference a variable.

What is the reason to change this? If you want to change this, we need to change all places. But this patch only changes some of them.

Changed as per suggestion

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And the commit message is to remove the "double quoting" problem. this kind of error/warning is not mentioned in the commit log. Is this a real warning or error?

Updated the statement with other issues as well. Thanks

@stale
Copy link

stale bot commented Aug 16, 2022

This issue/pull request has been marked as needs attention as it has been left pending without new activity for 4 days. Tagging @nkommuri @mehjoshi @huanghua78 for appropriate assignment. Sorry for the delay & Thank you for contributing to CORTX. We will get back to you as soon as possible.

@rkothiya rkothiya merged commit 2f26897 into Seagate:main Aug 19, 2022
kiwionly2 pushed a commit to kiwionly2/cortx-motr that referenced this pull request Aug 30, 2022
)

Problem: 
We see 1688 occurrences of the pattern, "Double quote to prevent globing and word splitting".
And issues like Use $(...) notation instead of legacy backticked ....

Solution:
We are putting the variable references in double-quotes.
Also fixed issues like Use $(...) notation instead of legacy backticked ....

Signed-off-by: Gaurav Gaur <[email protected]>
Co-authored-by: Venkatesh Balagani <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla-signed codacy Status: Waiting to be Reviewed PR is waiting for reviewers to review the PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants