[Scst-devel] SCST source code, checkpatch and sparse (5)

[Bart V. A. <bar...@gm...>]

The current checkpatch status for SCST (r584) is as follows (see also
http://sourceforge.net/mailarchive/message.php?msg_id=e2e108260810231212l109d645fi6da496e8b16e9094%40mail.gmail.com
for the previous report):

$ grep -E '^WARNING|^ERROR' checkpatch-2.6.27.7-output.txt | sort |
uniq -c | sort -rn
     32 WARNING: line over 80 characters
     20 ERROR: Macros with complex values should be enclosed in parenthesis
     12 WARNING: consider using strict_strtoul in preference to simple_strtoul
      3 WARNING: CVS style keyword markers, these will _not_ be updated
      1 WARNING: Use of volatile is usually wrong: see
Documentation/volatile-considered-harmful.txt
      1 WARNING: Use #include <linux/byteorder.h> instead of <asm/byteorder.h>
      1 WARNING: suspect code indent for conditional statements
      1 WARNING: do not add new typedefs
      1 ERROR: Macros with multiple statements should be enclosed in a
do - while loop

Regarding the checkpatch messages (in the same order as the above report):
* The only way to decrease the number of lines further that exceeds 80
columns is by introducing additional (small) functions.
* The errors reported on macros with complex values are false
positives (this has been acknowledged by the checkpatch authors). All
complaints are on defines like #define scst_sense_no_sense NO_SENSE,
0x00, 0
* Regarding simple_strtoul(): its usage in SCST is justified --
strict_strtoul() is strongly recommended for validating kernel
parameters. All uses of simple_strtoul() in the SCST source code is
for other purposes though.
* We know that the CVS style keyword markers won't be updated.
* The single usage of volatile in SCST is justified.
* I have tried to replace <asm/byteorder.h> by <linux/byteorder.h> but
this resulted in a compiler error.
* The single warning about suspect code for conditional statements is
a checkpatch bug. This has been acknowledged by the checkpatch
authors.
* The warning about the new typedef refers to 'typedef enum
dma_data_direction scst_data_direction'.
* The error about macros with multiple statements is a checkpatch bug,
which has been reported to the checkpatch authors and will be fixed in
a future checkpatch version.

The sparse output for the 2.6.27.7 kernel + SCST kernel patch can be
found below (latest git sparse revision, last modified in August
2008). My comments on the sparse output are as follows:
- Regarding the 'expected different context' warnings: I do not
understand why sparse reports these on the SCST source code -- all
SCST source code has been annotated properly by this time. I'm not
sure whether this is an issue with the annotations or an issue with
sparse.
- The warning that symbol _min1 shadows an earlier one can be ignored
-- this warning is due to nesting of the inline function min().


Sparse output (C=2):

  SYMLINK include/asm -> include/asm-x86

  WARNING: Symbol version dump
/sda6/home/bart/software/scst/regression-test-output-2008-11-26_20h02m18s/linux-2.6.27.7/Module.symvers
           is missing; modules will have no dependencies and modversions.

include/linux/mm.h:591:9: warning: potentially expensive pointer subtraction
include/linux/mm.h:591:9: warning: potentially expensive pointer subtraction
include/linux/mm.h:591:9: warning: potentially expensive pointer subtraction
include/linux/mm.h:591:9: warning: potentially expensive pointer subtraction
include/linux/mm.h:591:9: warning: potentially expensive pointer subtraction
include/linux/mm.h:591:9: warning: potentially expensive pointer subtraction
include/linux/mm.h:591:9: warning: potentially expensive pointer subtraction
include/linux/mm.h:591:9: warning: potentially expensive pointer subtraction
drivers/scst/scst_targ.c:3134:3: warning: context problem in
'scst_do_job_init': '_spin_unlock' expected different context
drivers/scst/scst_targ.c:3134:3:    context 'lock': wanted >= 1, got 0
drivers/scst/scst_targ.c:3197:19: warning: context problem in
'scst_init_cmd_thread': 'scst_do_job_init' expected different context
drivers/scst/scst_targ.c:3197:19:    default context: wanted >= 1, got 0
drivers/scst/scst_targ.c:3381:3: warning: context problem in
'scst_do_job_active': '_spin_unlock_irq' expected different context
drivers/scst/scst_targ.c:3381:3:    context 'lock': wanted >= 1, got 0
drivers/scst/scst_targ.c:3437:21: warning: context problem in
'scst_cmd_thread': 'scst_do_job_active' expected different context
drivers/scst/scst_targ.c:3437:21:    context 'cmd_list_lock': wanted >= 1, got 0
drivers/scst/scst_targ.c:3467:20: warning: context problem in
'scst_cmd_tasklet': 'scst_do_job_active' expected different context
drivers/scst/scst_targ.c:3467:20:    context 'cmd_list_lock': wanted >= 1, got 0
include/linux/mm.h:591:9: warning: potentially expensive pointer subtraction
include/linux/mm.h:591:9: warning: potentially expensive pointer subtraction
include/linux/mm.h:591:9: warning: potentially expensive pointer subtraction
include/linux/mm.h:591:9: warning: potentially expensive pointer subtraction
include/linux/mm.h:591:9: warning: potentially expensive pointer subtraction
include/linux/mm.h:591:9: warning: potentially expensive pointer subtraction
include/linux/mm.h:591:9: warning: potentially expensive pointer subtraction
drivers/scst/scst_mem.c:64:26: warning: potentially expensive pointer
subtraction
drivers/scst/scst_mem.c:77:9: warning: potentially expensive pointer subtraction
drivers/scst/scst_mem.c:90:9: warning: potentially expensive pointer subtraction
drivers/scst/scst_mem.c:457:3: warning: context problem in
'sgv_pool_oom_free_objs': '_spin_unlock_bh' expected different context
drivers/scst/scst_mem.c:457:3:    context 'lock': wanted >= 1, got 0
drivers/scst/scst_mem.c:480:33: warning: context problem in
'sgv_pool_hiwmk_check': 'sgv_pool_oom_free_objs' expected different
context
drivers/scst/scst_mem.c:480:33:    default context: wanted >= 1, got 0
include/linux/mm.h:591:9: warning: potentially expensive pointer subtraction
include/linux/mm.h:591:9: warning: potentially expensive pointer subtraction
include/linux/mm.h:591:9: warning: potentially expensive pointer subtraction
include/linux/mm.h:591:9: warning: potentially expensive pointer subtraction
include/linux/mm.h:591:9: warning: potentially expensive pointer subtraction
include/linux/mm.h:591:9: warning: potentially expensive pointer subtraction
include/linux/mm.h:591:9: warning: potentially expensive pointer subtraction
include/linux/mm.h:591:9: warning: potentially expensive pointer subtraction
include/linux/mm.h:591:9: warning: potentially expensive pointer subtraction
include/linux/mm.h:591:9: warning: potentially expensive pointer subtraction
include/linux/mm.h:591:9: warning: potentially expensive pointer subtraction
include/linux/mm.h:591:9: warning: potentially expensive pointer subtraction
include/linux/mm.h:591:9: warning: potentially expensive pointer subtraction
include/linux/mm.h:591:9: warning: potentially expensive pointer subtraction
include/linux/mm.h:591:9: warning: potentially expensive pointer subtraction
include/linux/mm.h:591:9: warning: potentially expensive pointer subtraction
drivers/scst/dev_handlers/scst_user.c:1503:3: warning: context problem
in 'dev_user_process_scst_commands': '_spin_unlock_irq' expected
different context
drivers/scst/dev_handlers/scst_user.c:1503:3:    context 'lock':
wanted >= 1, got 0
drivers/scst/dev_handlers/scst_user.c:1538:5: warning: context problem
in '__dev_user_get_next_cmd': '_spin_unlock_irq' expected different
context
drivers/scst/dev_handlers/scst_user.c:1538:5:    context 'lock':
wanted >= 1, got 0
drivers/scst/dev_handlers/scst_user.c:1611:33: warning: context
problem in 'dev_user_get_next_cmd': 'dev_user_process_scst_commands'
expected different context
drivers/scst/dev_handlers/scst_user.c:1611:33:    default context:
wanted >= 1, got 0
drivers/scst/dev_handlers/scst_user.c:1865:4: warning: context problem
in 'dev_user_unjam_cmd': '_spin_unlock_irqrestore' expected different
context
drivers/scst/dev_handlers/scst_user.c:1865:4:    context 'lock':
wanted >= 1, got 0
drivers/scst/dev_handlers/scst_user.c:1971:32: warning: context
imbalance in 'dev_user_unjam_dev': __context__ statement expected
different context
drivers/scst/dev_handlers/scst_user.c:1971:32:    context '<noident>':
wanted >= 0, got -1
drivers/scst/dev_handlers/scst_user.c:2037:23: warning: context
problem in 'dev_user_abort_ready_commands': 'dev_user_unjam_cmd'
expected different context
drivers/scst/dev_handlers/scst_user.c:2037:23:    default context:
wanted >= 1, got 0
drivers/scst/dev_handlers/scst_user.c:2783:21: warning: context
problem in 'dev_user_process_cleanup': 'dev_user_unjam_dev' expected
different context
drivers/scst/dev_handlers/scst_user.c:2783:21:    default context:
wanted >= 1, got 0
include/linux/mm.h:591:9: warning: potentially expensive pointer subtraction
include/linux/mm.h:591:9: warning: potentially expensive pointer subtraction
include/linux/mm.h:591:9: warning: potentially expensive pointer subtraction
include/linux/mm.h:591:9: warning: potentially expensive pointer subtraction
include/linux/mm.h:591:9: warning: potentially expensive pointer subtraction
include/linux/mm.h:591:9: warning: potentially expensive pointer subtraction
include/linux/mm.h:591:9: warning: potentially expensive pointer subtraction
drivers/scst/iscsi-scst/iscsi.c:2135:6: warning: dereference of
noderef expression
drivers/scst/iscsi-scst/iscsi.c:2136:6: warning: dereference of
noderef expression
drivers/scst/iscsi-scst/iscsi.c:2757:2: warning: context problem in
'iscsi_check_send_delayed_tm_resp': '_spin_unlock' expected different
context
drivers/scst/iscsi-scst/iscsi.c:2757:2:    context 'lock': wanted >= 1, got 0
drivers/scst/iscsi-scst/nthread.c:1073:4: error: incompatible types in
comparison expression (different address spaces)
drivers/scst/iscsi-scst/digest.c:74:11: warning: symbol '_min1'
shadows an earlier one
drivers/scst/iscsi-scst/digest.c:74:11: originally declared here
include/linux/mm.h:591:9: warning: potentially expensive pointer subtraction
  Building modules, stage 2.

Bart.