Project

General

Profile

Bug #7229

dangerous macros in sdev_impl.h

Added by Daniel Kimmel almost 4 years ago. Updated almost 4 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
zfs - Zettabyte File System
Start date:
2016-07-28
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
needs-triage
Gerrit CR:

Description

While trying to do a nondebug build, I ran into compiler errors when building uts/intel/dev. These errors do not occur during a nondebug nightly build because the dev module is normally built with "-Wno-unused -Wno-empty-body", but for some reason, my build was not specifying these flags.
I investigated the compiler errors and several of them are truly horrifying, as they lead to some code not being executed when it should. The errors are due to use of the unsafe-at-any-speed sdcmn_err*() macros:

#ifdef DEBUG
#define sdcmn_err(args) if (sdev_debug & SDEV_DEBUG) printf args
#else
#define sdcmn_err(args) /* does nothing */
#endif

Typical usage is:

if (something)
sdcmn_err("fmt", arg, arg, arg);
next_statement;

On debug builds, next_statement is executed unconditionally, but on nondebug builds, next_statement is only executed if "something".
The problem has existed at least since 2006.
We should fix these macros to be safe to use in "if" statements, and stop squelching the compiler error messages. These macros do not directly cause a malfunction, so this is just code cleanup to make them less prone to misuse.

Error messages:

../../common/fs/dev/sdev_vnops.c: In function 'sdev_rmdir':
../../common/fs/dev/sdev_vnops.c:1372: error: suggest braces around empty body in an 'if' statement [-Wempty-body]

../../common/fs/dev/sdev_ptsops.c: In function 'devpts_lookup':
../../common/fs/dev/sdev_ptsops.c:297: error: unused variable 'rvp' [-Wunused-variable]

../../common/fs/dev/sdev_zvolops.c: In function 'devzvol_validate':
../../common/fs/dev/sdev_zvolops.c:266: error: unused variable 'nm' [-Wunused-variable]
../../common/fs/dev/sdev_profile.c: In function 'prof_mknode':
../../common/fs/dev/sdev_profile.c:174: error: suggest braces around empty body in an 'if' statement [-Wempty-body]

../../common/fs/dev/sdev_profile.c: In function 'prof_filldir':
../../common/fs/dev/sdev_profile.c:697: error: suggest braces around empty body in an 'if' statement [-Wempty-bod

../../common/fs/dev/sdev_ipnetops.c: In function 'devipnet_lookup':
../../common/fs/dev/sdev_ipnetops.c:111: error: unused variable 'rvp' [-Wunused-variable]

../../common/fs/dev/sdev_vtops.c: In function 'devvt_lookup':
../../common/fs/dev/sdev_vtops.c:190: error: unused variable 'rvp' [-Wunused-variable]

History

#1

Updated by Electric Monk almost 4 years ago

  • % Done changed from 0 to 100
  • Status changed from New to Closed

git commit 6c60cf15430935d55cc9424e9f4c70acd8592df3

commit  6c60cf15430935d55cc9424e9f4c70acd8592df3
Author: Matthew Ahrens <mahrens@delphix.com>
Date:   2016-08-02T16:56:32.000Z

    7229 dangerous macros in sdev_impl.h
    Reviewed by: Prakash Surya <prakash.surya@delphix.com>
    Reviewed by: Eric Diven <eric.diven@delphix.com>
    Reviewed by: Alex Reece <alex@delphix.com>
    Reviewed by: Adam Leventhal <ahl@delphix.com>
    Reviewed by: Chris Siden <christopher.siden@delphix.com>
    Reviewed by: Igor Kozhukhov <ikozhukhov@gmail.com>
    Approved by: Robert Mustacchi <rm@joyent.com>

Also available in: Atom PDF