Opened 2 months ago

Last modified 7 hours ago

#29073 merge_ready defect

shellcheck: linux-tor-prio.sh issues

Reported by: rl1987 Owned by: rl1987
Priority: Medium Milestone: Tor: 0.4.1.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: catalyst Sponsor:

Description

In linux-tor-prio.sh line 98:
MTU=1500
^-- SC2034: MTU appears unused. Verify use (or export if used externally).


In linux-tor-prio.sh line 108:
BDP=$(expr $RTT_LATENCY \* $RATE_UP / $AVG_PKT)
      ^-- SC2003: expr is antiquated. Consider rewriting this using $((..)), ${} or [[ ]].


In linux-tor-prio.sh line 113:
BDP=$(expr $BDP / 4)
      ^-- SC2003: expr is antiquated. Consider rewriting this using $((..)), ${} or [[ ]].
           ^-- SC2086: Double quote to prevent globbing and word splitting.


In linux-tor-prio.sh line 151:
ip link set dev $DEV qlen $BDP
                          ^-- SC2086: Double quote to prevent globbing and word splitting.


In linux-tor-prio.sh line 161:
tc class add dev $DEV parent 1:1 classid 1:20 htb rate $(expr $RATE_UP - $RATE_UP_TOR)kbit ceil ${RATE_UP}kbit prio 0
                                                       ^-- SC2046: Quote this to prevent word splitting.
                                                         ^-- SC2003: expr is antiquated. Consider rewriting this using $((..)), ${} or [[ ]].


In linux-tor-prio.sh line 162:
tc class add dev $DEV parent 1:1 classid 1:21 htb rate $[$RATE_UP_TOR]kbit ceil ${RATE_UP_TOR_CEIL}kbit prio 10
                                                       ^-- SC2007: Use $((..)) instead of deprecated $[..]


In linux-tor-prio.sh line 165:
tc qdisc add dev $DEV parent 1:20 handle 20: pfifo limit $BDP
                                                         ^-- SC2086: Double quote to prevent globbing and word splitting.


In linux-tor-prio.sh line 166:
tc qdisc add dev $DEV parent 1:21 handle 21: pfifo limit $BDP
                                                         ^-- SC2086: Double quote to prevent globbing and word splitting.


In linux-tor-prio.sh line 179:
if [ ""$TOR_IP == "" ]
       ^-- SC2086: Double quote to prevent globbing and word splitting.


In linux-tor-prio.sh line 182:
	iptables -t mangle -A TORSHAPER-OUT -m owner --uid-owner $TOR_UID -j MARK --set-mark 21
                                                                 ^-- SC2086: Double quote to prevent globbing and word splitting.


In linux-tor-prio.sh line 185:
	iptables -t mangle -A TORSHAPER-OUT -s $TOR_IP -j MARK --set-mark 21
                                               ^-- SC2086: Double quote to prevent globbing and word splitting.

Child Tickets

Change History (10)

comment:1 Changed 5 weeks ago by rl1987

Owner: set to rl1987
Status: newaccepted

comment:2 Changed 5 weeks ago by rl1987

Status: acceptedneeds_review

comment:3 Changed 5 weeks ago by dgoulet

Reviewer: catalyst

comment:4 Changed 5 weeks ago by nickm

Milestone: Tor: 0.4.1.x-final

comment:5 Changed 3 weeks ago by catalyst

Status: needs_reviewneeds_revision

Looks good by visual inspection. Please split the formatting change into a separate commit, as noted on github. I'll try to see if anyone is willing to test it out.

comment:6 Changed 2 weeks ago by rl1987

Status: needs_revisionneeds_review

comment:7 Changed 8 days ago by rl1987

See also: #29434

Last edited 8 days ago by rl1987 (previous) (diff)

comment:8 in reply to:  6 Changed 8 hours ago by catalyst

Status: needs_reviewneeds_revision

Thanks! Could you please clean up the commit history a bit instead of having a discrete commit to revert earlier changes? Or if you'd rather we cleaned it up, that is also fine but might take longer.

comment:9 Changed 8 hours ago by rl1987

Status: needs_revisionneeds_review

New pull request with cleaner git history: https://github.com/torproject/tor/pull/804

comment:10 in reply to:  9 Changed 7 hours ago by catalyst

Status: needs_reviewmerge_ready

Replying to rl1987:

New pull request with cleaner git history: https://github.com/torproject/tor/pull/804

Thanks! Looks good!

Note: See TracTickets for help on using tickets.