Endless loop in zpool_do_remove() on platforms with unsigned char

[loli10K]

Motivation and Context

On systems where "char" is an unsigned type the value returned by getopt() will never be negative (-1), leading to an endlees loop.

root@linux:~# uname -m
armv7l
root@linux:~# truncate -s 1024m /var/tmp/1
root@linux:~# truncate -s 1024m /var/tmp/2
root@linux:~# zpool create testpool /var/tmp/1 cache /var/tmp/2
root@linux:~# modinfo -F version zfs
0.8.0-rc5
root@linux:~# zpool remove testpool /var/tmp/2
<hang>

zpool is now just wasting CPU cycles:

top - 17:59:31 up 12:22,  4 users,  load average: 1.68, 1.30, 1.42           
Tasks: 355 total,   3 running, 351 sleeping,   0 stopped,   1 zombie       
%Cpu(s): 28.7 us,  4.6 sy,  0.0 ni, 66.6 id,  0.0 wa,  0.0 hi,  0.0 si,  0.0 st    
KiB Mem :  2024340 total,   633692 free,  1167016 used,   223632 buff/cache     
KiB Swap:        0 total,        0 free,        0 used.   803384 avail Mem 
                                                                            
  PID USER      PR  NI    VIRT    RES    SHR S  %CPU %MEM     TIME+ COMMAND                                                                                                                                        
  483 root      20   0    7328   1536   1236 R 100.0  0.1   0:27.60 zpool        

Description

Change char c to int c

How Has This Been Tested?

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Performance enhancement (non-breaking change which improves efficiency)
• Code cleanup (non-breaking change which makes code smaller or more readable)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation (a change to man pages or other documentation)

Checklist:

• My code follows the ZFS on Linux code style requirements.
• I have updated the documentation accordingly.
• I have read the contributing document.
• I have added tests to cover my changes.
• All new and existing tests passed.
• All commit messages are properly formatted and contain Signed-off-by.

[codecov]

Codecov Report

Merging #8789 into master will decrease coverage by 0.06%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #8789      +/-   ##
==========================================
- Coverage   78.75%   78.68%   -0.07%     
==========================================
  Files         382      381       -1     
  Lines      117809   117797      -12     
==========================================
- Hits        92776    92691      -85     
- Misses      25033    25106      +73

Flag	Coverage Δ
#kernel	79.32% <ø> (+0.01%)	⬆️
#user	67.08% <ø> (-0.42%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c3e5907...c7b2812. Read the comment docs.

[chrisrd]

Whilst you're there, may as well fix up the other places where we don't have getopt() returning an int? E.g.:

$ grep -rEn '(int|char).*[[:space:]]c[;,]|getopt\(' cmd | grep -B1 getopt | less +/char
cmd/zstreamdump/zstreamdump.c:240:      char c;
cmd/zstreamdump/zstreamdump.c:254:      while ((c = getopt(argc, argv, ":vCd")) != -1) {
--
cmd/zpool/zpool_main.c:975:     char c;
cmd/zpool/zpool_main.c:980:     while ((c = getopt(argc, argv, "nps")) != -1) {
--
cmd/zfs/zfs_main.c:2241:        signed char c;
cmd/zfs/zfs_main.c:2245:        while ((c = getopt(argc, argv, "rvV:a")) != -1) {
--
cmd/zfs/zfs_main.c:3935:        signed char c;
cmd/zfs/zfs_main.c:3946:        while ((c = getopt(argc, argv, "ro:")) != -1) {

[behlendorf]

A typo here caused the build failures.

[behlendorf]

@chrisrd would you mind reviewing this again.

[ikozhukhov]

i have tested int c; and it is working as well, thanks for this work.