From fffc091b609cc8461cdb80cb9b4e96d3bfc2aa37 Mon Sep 17 00:00:00 2001 From: "Gabriel A. Devenyi" Date: Sat, 15 Aug 2026 00:30:44 -0400 Subject: [PATCH] BUG: Parenthesize macro arguments and replacement lists Clears all 155 bugprone-macro-parentheses findings: 152 in nifti2/nifti_tool.c, 2 in niftilib/nifti1_tool.c, 1 in fsliolib/fslio.c. An unparenthesized argument takes the precedence of whatever the caller passed. NT_FILL(..., rv) expands to `rv = ...` and NT_MAT33_TO_MAT44 to `m44.m[0][0] = ...`, which break for an argument that is not already a postfix expression. Every current caller passes a plain identifier, a literal or `nim->sto_xyz`, so nothing observable moves. The NT_DCONVERT dtype and stype parameters stay bare behind a NOLINT fence: they name types, so `(dtype) * pd = dptr;` would be a cast expression rather than a declaration. FSL_RADIOLOGICAL is in an installed header, so its replacement list changes from -1 to (-1); its value does not. --- fsliolib/fslio.h | 2 +- nifti2/nifti_tool.h | 116 +++++++++++++++++++++-------------------- niftilib/nifti1_tool.h | 4 +- 3 files changed, 63 insertions(+), 59 deletions(-) diff --git a/fsliolib/fslio.h b/fsliolib/fslio.h index ab1edbd5..1d384d79 100644 --- a/fsliolib/fslio.h +++ b/fsliolib/fslio.h @@ -135,7 +135,7 @@ extern "C" { #define FSL_TYPE_MINC_GZ 104 /* @} */ -#define FSL_RADIOLOGICAL -1 +#define FSL_RADIOLOGICAL (-1) #define FSL_NEUROLOGICAL 1 diff --git a/nifti2/nifti_tool.h b/nifti2/nifti_tool.h index ba57369d..9cfa4744 100644 --- a/nifti2/nifti_tool.h +++ b/nifti2/nifti_tool.h @@ -124,14 +124,17 @@ typedef struct { /* call fill_field() for a single type, name and number of elements */ /* nstr is the base struct, and fldp is a field pointer */ #define NT_FILL(nstr,fldp,type,name,num,rv) do{ \ - rv=fill_field(fldp,type,offsetof(nstr,name),num,#name); \ - fldp++; } while (0) + (rv)=fill_field(fldp,type,offsetof(nstr,name),num,#name); \ + (fldp)++; } while (0) #define NT_MAKE_IM_NAME "MAKE_IM" /* ================================================================= */ /* data conversionn operations */ +/* dtype and stype are type names, so they cannot be parenthesized: + `(dtype) * pd = dptr;` is a cast expression, not a declaration. */ +/* NOLINTBEGIN(bugprone-macro-parentheses) */ /* -------------------------------------------- */ /* copy from src to dest, changing type enroute */ /* dtype, dptr : destination type and pointer */ @@ -157,14 +160,15 @@ typedef struct { stype * ps = sptr; \ int64_t index; \ /* init bounds with first */ \ - failure = 0; \ + (failure) = 0; \ for(index=0; index<(nvals); index++) { \ *pd = (dtype)*ps; \ /* fail when we cannot invert */ \ - if( !failure && *ps != (stype)*pd ) \ - failure = 1; \ + if( !(failure) && *ps != (stype)*pd ) \ + (failure) = 1; \ pd++; ps++; \ } } while(0) +/* NOLINTEND(bugprone-macro-parentheses) */ /* ================================================================= */ @@ -172,74 +176,74 @@ typedef struct { /* (macros allow them to apply to either mat44 or dmat44) */ /* fill MAT44 with MAT33 fields, then pad with 0.0 and a 1.0 at 3,3 */ -#define NT_MAT33_TO_MAT44(m33, m44) do { \ - m44.m[0][0] = m33.m[0][0]; m44.m[0][1] = m33.m[0][1]; \ - m44.m[0][2] = m33.m[0][2]; \ - m44.m[1][0] = m33.m[1][0]; m44.m[1][1] = m33.m[1][1]; \ - m44.m[1][2] = m33.m[1][2]; \ - m44.m[2][0] = m33.m[2][0]; m44.m[2][1] = m33.m[2][1]; \ - m44.m[2][2] = m33.m[2][2]; \ - /* and fill out the 4x4 mat */ \ - m44.m[0][3] = m44.m[1][3] = m44.m[2][3] = 0.0; \ - m44.m[3][0] = m44.m[3][1] = m44.m[3][2] = 0.0; \ - m44.m[3][3] = 1.0; \ +#define NT_MAT33_TO_MAT44(m33, m44) do { \ + (m44).m[0][0] = (m33).m[0][0]; (m44).m[0][1] = (m33).m[0][1]; \ + (m44).m[0][2] = (m33).m[0][2]; \ + (m44).m[1][0] = (m33).m[1][0]; (m44).m[1][1] = (m33).m[1][1]; \ + (m44).m[1][2] = (m33).m[1][2]; \ + (m44).m[2][0] = (m33).m[2][0]; (m44).m[2][1] = (m33).m[2][1]; \ + (m44).m[2][2] = (m33).m[2][2]; \ + /* and fill out the 4x4 mat */ \ + (m44).m[0][3] = (m44).m[1][3] = (m44).m[2][3] = 0.0; \ + (m44).m[3][0] = (m44).m[3][1] = (m44).m[3][2] = 0.0; \ + (m44).m[3][3] = 1.0; \ } while(0) /* fill MAT33 with initial subset of MAT44 fields */ -#define NT_MAT44_TO_MAT33(m44, m33) do { \ - m33.m[0][0] = m44.m[0][0]; m33.m[0][1] = m44.m[0][1]; \ - m33.m[0][2] = m44.m[0][2]; \ - m33.m[1][0] = m44.m[1][0]; m33.m[1][1] = m44.m[1][1]; \ - m33.m[1][2] = m44.m[1][2]; \ - m33.m[2][0] = m44.m[2][0]; m33.m[2][1] = m44.m[2][1]; \ - m33.m[2][2] = m44.m[2][2]; \ +#define NT_MAT44_TO_MAT33(m44, m33) do { \ + (m33).m[0][0] = (m44).m[0][0]; (m33).m[0][1] = (m44).m[0][1]; \ + (m33).m[0][2] = (m44).m[0][2]; \ + (m33).m[1][0] = (m44).m[1][0]; (m33).m[1][1] = (m44).m[1][1]; \ + (m33).m[1][2] = (m44).m[1][2]; \ + (m33).m[2][0] = (m44).m[2][0]; (m33).m[2][1] = (m44).m[2][1]; \ + (m33).m[2][2] = (m44).m[2][2]; \ } while(0) /* subtract 2 mat44 matrices */ #define NT_MAT44_SUBTRACT(mout, min0, min1) do { \ - mout.m[0][0] = min0.m[0][0] - min1.m[0][0]; \ - mout.m[0][1] = min0.m[0][1] - min1.m[0][1]; \ - mout.m[0][2] = min0.m[0][2] - min1.m[0][2]; \ - mout.m[0][3] = min0.m[0][3] - min1.m[0][3]; \ - mout.m[1][0] = min0.m[1][0] - min1.m[1][0]; \ - mout.m[1][1] = min0.m[1][1] - min1.m[1][1]; \ - mout.m[1][2] = min0.m[1][2] - min1.m[1][2]; \ - mout.m[1][3] = min0.m[1][3] - min1.m[1][3]; \ - mout.m[2][0] = min0.m[2][0] - min1.m[2][0]; \ - mout.m[2][1] = min0.m[2][1] - min1.m[2][1]; \ - mout.m[2][2] = min0.m[2][2] - min1.m[2][2]; \ - mout.m[2][3] = min0.m[2][3] - min1.m[2][3]; \ - mout.m[3][0] = min0.m[3][0] - min1.m[3][0]; \ - mout.m[3][1] = min0.m[3][1] - min1.m[3][1]; \ - mout.m[3][2] = min0.m[3][2] - min1.m[3][2]; \ - mout.m[3][3] = min0.m[3][3] - min1.m[3][3]; \ + (mout).m[0][0] = (min0).m[0][0] - (min1).m[0][0]; \ + (mout).m[0][1] = (min0).m[0][1] - (min1).m[0][1]; \ + (mout).m[0][2] = (min0).m[0][2] - (min1).m[0][2]; \ + (mout).m[0][3] = (min0).m[0][3] - (min1).m[0][3]; \ + (mout).m[1][0] = (min0).m[1][0] - (min1).m[1][0]; \ + (mout).m[1][1] = (min0).m[1][1] - (min1).m[1][1]; \ + (mout).m[1][2] = (min0).m[1][2] - (min1).m[1][2]; \ + (mout).m[1][3] = (min0).m[1][3] - (min1).m[1][3]; \ + (mout).m[2][0] = (min0).m[2][0] - (min1).m[2][0]; \ + (mout).m[2][1] = (min0).m[2][1] - (min1).m[2][1]; \ + (mout).m[2][2] = (min0).m[2][2] - (min1).m[2][2]; \ + (mout).m[2][3] = (min0).m[2][3] - (min1).m[2][3]; \ + (mout).m[3][0] = (min0).m[3][0] - (min1).m[3][0]; \ + (mout).m[3][1] = (min0).m[3][1] - (min1).m[3][1]; \ + (mout).m[3][2] = (min0).m[3][2] - (min1).m[3][2]; \ + (mout).m[3][3] = (min0).m[3][3] - (min1).m[3][3]; \ } while(0) /* subtract 2 mat33 matrices */ #define NT_MAT33_SUBTRACT(mout, min0, min1) do { \ - mout.m[0][0] = min0.m[0][0] - min1.m[0][0]; \ - mout.m[0][1] = min0.m[0][1] - min1.m[0][1]; \ - mout.m[0][2] = min0.m[0][2] - min1.m[0][2]; \ - mout.m[1][0] = min0.m[1][0] - min1.m[1][0]; \ - mout.m[1][1] = min0.m[1][1] - min1.m[1][1]; \ - mout.m[1][2] = min0.m[1][2] - min1.m[1][2]; \ - mout.m[2][0] = min0.m[2][0] - min1.m[2][0]; \ - mout.m[2][1] = min0.m[2][1] - min1.m[2][1]; \ - mout.m[2][2] = min0.m[2][2] - min1.m[2][2]; \ + (mout).m[0][0] = (min0).m[0][0] - (min1).m[0][0]; \ + (mout).m[0][1] = (min0).m[0][1] - (min1).m[0][1]; \ + (mout).m[0][2] = (min0).m[0][2] - (min1).m[0][2]; \ + (mout).m[1][0] = (min0).m[1][0] - (min1).m[1][0]; \ + (mout).m[1][1] = (min0).m[1][1] - (min1).m[1][1]; \ + (mout).m[1][2] = (min0).m[1][2] - (min1).m[1][2]; \ + (mout).m[2][0] = (min0).m[2][0] - (min1).m[2][0]; \ + (mout).m[2][1] = (min0).m[2][1] - (min1).m[2][1]; \ + (mout).m[2][2] = (min0).m[2][2] - (min1).m[2][2]; \ } while(0) /* fill with identity matrix */ -#define NT_MAT44_SET_TO_IDENTITY(M) do { \ - M.m[0][0] = 1.0; M.m[0][1] = 0.0; M.m[0][2] = 0.0; M.m[0][3] = 0.0; \ - M.m[1][0] = 0.0; M.m[1][1] = 1.0; M.m[1][2] = 0.0; M.m[1][3] = 0.0; \ - M.m[2][0] = 0.0; M.m[2][1] = 0.0; M.m[2][2] = 1.0; M.m[2][3] = 0.0; \ - M.m[3][0] = 0.0; M.m[3][1] = 0.0; M.m[3][2] = 0.0; M.m[3][3] = 1.0; \ +#define NT_MAT44_SET_TO_IDENTITY(M) do { \ + (M).m[0][0] = 1.0; (M).m[0][1] = 0.0; (M).m[0][2] = 0.0; (M).m[0][3] = 0.0; \ + (M).m[1][0] = 0.0; (M).m[1][1] = 1.0; (M).m[1][2] = 0.0; (M).m[1][3] = 0.0; \ + (M).m[2][0] = 0.0; (M).m[2][1] = 0.0; (M).m[2][2] = 1.0; (M).m[2][3] = 0.0; \ + (M).m[3][0] = 0.0; (M).m[3][1] = 0.0; (M).m[3][2] = 0.0; (M).m[3][3] = 1.0; \ } while(0) #define NT_MAT33_SET_TO_IDENTITY(M) do { \ - M.m[0][0] = 1.0; M.m[0][1] = 0.0; M.m[0][2] = 0.0; \ - M.m[1][0] = 0.0; M.m[1][1] = 1.0; M.m[1][2] = 0.0; \ - M.m[2][0] = 0.0; M.m[2][1] = 0.0; M.m[2][2] = 1.0; \ + (M).m[0][0] = 1.0; (M).m[0][1] = 0.0; (M).m[0][2] = 0.0; \ + (M).m[1][0] = 0.0; (M).m[1][1] = 1.0; (M).m[1][2] = 0.0; \ + (M).m[2][0] = 0.0; (M).m[2][1] = 0.0; (M).m[2][2] = 1.0; \ } while(0) diff --git a/niftilib/nifti1_tool.h b/niftilib/nifti1_tool.h index b099924a..1670a75a 100644 --- a/niftilib/nifti1_tool.h +++ b/niftilib/nifti1_tool.h @@ -88,8 +88,8 @@ typedef struct { /* call fill_field() for a single type, name and number of elements */ /* nstr is the base struct, and fldp is a field pointer */ #define NT_FILL(nstr,fldp,type,name,num,rv) do{ \ - rv=fill_field(fldp,type,offsetof(nstr,name),num,#name); \ - fldp++; } while (0) + (rv)=fill_field(fldp,type,offsetof(nstr,name),num,#name); \ + (fldp)++; } while (0) #define NT_MAKE_IM_NAME "MAKE_IM"