Skip to content

Commit

Permalink
Modernize macros to use do { } while (0)
Browse files Browse the repository at this point in the history 
This PR introduces no functional changes. It attempts to change all
macros currently using `{ }` or some variant of that to to
`do { } while (0)`, and introduces trailing `;` where necessary.
There were no bugs found during this migration.

The bug in Visual Studios warning on this has been fixed since VS2015.
Additionally, we have several instances of `do { } while (0)` which have
been present for several releases, so we don't have to worry about
breaking peoples builds.

Fixes Issue facebook#3830.
  • Loading branch information
@terrelln
Nick Terrell authored and terrelln committed Nov 22, 2023
1 parent 6b3d12f commit 8193250
Show file tree
Hide file tree
Showing 12 changed files with 207 additions and 169 deletions.
33 changes: 17 additions & 16 deletions lib/common/compiler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,8 @@
/* prefetch
* can be disabled, by declaring NO_PREFETCH build macro */
#if defined(NO_PREFETCH)
# define PREFETCH_L1(ptr) (void)(ptr) /* disabled */
# define PREFETCH_L2(ptr) (void)(ptr) /* disabled */
# define PREFETCH_L1(ptr) do { (void)(ptr); } while (0) /* disabled */
# define PREFETCH_L2(ptr) do { (void)(ptr); } while (0) /* disabled */
#else
# if defined(_MSC_VER) && (defined(_M_X64) || defined(_M_I86)) && !defined(_M_ARM64EC) /* _mm_prefetch() is not defined outside of x86/x64 */
# include <mmintrin.h> /* https://msdn.microsoft.com/fr-fr/library/84szxsww(v=vs.90).aspx */
Expand All @@ -143,24 +143,25 @@
# define PREFETCH_L1(ptr) __builtin_prefetch((ptr), 0 /* rw==read */, 3 /* locality */)
# define PREFETCH_L2(ptr) __builtin_prefetch((ptr), 0 /* rw==read */, 2 /* locality */)
# elif defined(__aarch64__)
# define PREFETCH_L1(ptr) __asm__ __volatile__("prfm pldl1keep, %0" ::"Q"(*(ptr)))
# define PREFETCH_L2(ptr) __asm__ __volatile__("prfm pldl2keep, %0" ::"Q"(*(ptr)))
# define PREFETCH_L1(ptr) do { __asm__ __volatile__("prfm pldl1keep, %0" ::"Q"(*(ptr))); } while (0)
# define PREFETCH_L2(ptr) do { __asm__ __volatile__("prfm pldl2keep, %0" ::"Q"(*(ptr))); } while (0)
# else
# define PREFETCH_L1(ptr) (void)(ptr) /* disabled */
# define PREFETCH_L2(ptr) (void)(ptr) /* disabled */
# define PREFETCH_L1(ptr) do { (void)(ptr); } while (0) /* disabled */
# define PREFETCH_L2(ptr) do { (void)(ptr); } while (0) /* disabled */
# endif
#endif /* NO_PREFETCH */

#define CACHELINE_SIZE 64

#define PREFETCH_AREA(p, s) { \
const char* const _ptr = (const char*)(p); \
size_t const _size = (size_t)(s); \
size_t _pos; \
for (_pos=0; _pos<_size; _pos+=CACHELINE_SIZE) { \
PREFETCH_L2(_ptr + _pos); \
} \
}
#define PREFETCH_AREA(p, s) \
do { \
const char* const _ptr = (const char*)(p); \
size_t const _size = (size_t)(s); \
size_t _pos; \
for (_pos=0; _pos<_size; _pos+=CACHELINE_SIZE) { \
PREFETCH_L2(_ptr + _pos); \
} \
} while (0)

/* vectorization
* older GCC (pre gcc-4.3 picked as the cutoff) uses a different syntax,
Expand Down Expand Up @@ -189,9 +190,9 @@
#endif

#if __has_builtin(__builtin_unreachable) || (defined(__GNUC__) && (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 5)))
# define ZSTD_UNREACHABLE { assert(0), __builtin_unreachable(); }
# define ZSTD_UNREACHABLE do { assert(0), __builtin_unreachable(); } while (0)
#else
# define ZSTD_UNREACHABLE { assert(0); }
# define ZSTD_UNREACHABLE do { assert(0); } while (0)
#endif

/* disable warnings */
Expand Down
27 changes: 16 additions & 11 deletions lib/common/debug.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,18 +85,23 @@ extern int g_debuglevel; /* the variable is only declared,
It's useful when enabling very verbose levels
on selective conditions (such as position in src) */

# define RAWLOG(l, ...) { \
if (l<=g_debuglevel) { \
ZSTD_DEBUG_PRINT(__VA_ARGS__); \
} }
# define DEBUGLOG(l, ...) { \
if (l<=g_debuglevel) { \
ZSTD_DEBUG_PRINT(__FILE__ ": " __VA_ARGS__); \
ZSTD_DEBUG_PRINT(" \n"); \
} }
# define RAWLOG(l, ...) \
do { \
if (l<=g_debuglevel) { \
ZSTD_DEBUG_PRINT(__VA_ARGS__); \
} \
} while (0)

# define DEBUGLOG(l, ...) \
do { \
if (l<=g_debuglevel) { \
ZSTD_DEBUG_PRINT(__FILE__ ": " __VA_ARGS__); \
ZSTD_DEBUG_PRINT(" \n"); \
} \
} while (0)
#else
# define RAWLOG(l, ...) {} /* disabled */
# define DEBUGLOG(l, ...) {} /* disabled */
# define RAWLOG(l, ...) do { } while (0) /* disabled */
# define DEBUGLOG(l, ...) do { } while (0) /* disabled */
#endif


Expand Down
81 changes: 45 additions & 36 deletions lib/common/error_private.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,13 @@ ERR_STATIC unsigned ERR_isError(size_t code) { return (code > ERROR(maxCode)); }
ERR_STATIC ERR_enum ERR_getErrorCode(size_t code) { if (!ERR_isError(code)) return (ERR_enum)0; return (ERR_enum) (0-code); }

/* check and forward error code */
#define CHECK_V_F(e, f) size_t const e = f; if (ERR_isError(e)) return e
#define CHECK_F(f) { CHECK_V_F(_var_err__, f); }
#define CHECK_V_F(e, f) \
size_t const e = f; \
do { \
if (ERR_isError(e)) \
return e; \
} while (0)
#define CHECK_F(f) do { CHECK_V_F(_var_err__, f); } while (0)


/*-****************************************
Expand Down Expand Up @@ -95,10 +100,12 @@ void _force_has_format_string(const char *format, ...) {
* We want to force this function invocation to be syntactically correct, but
* we don't want to force runtime evaluation of its arguments.
*/
#define _FORCE_HAS_FORMAT_STRING(...) \
if (0) { \
_force_has_format_string(__VA_ARGS__); \
}
#define _FORCE_HAS_FORMAT_STRING(...) \
do { \
if (0) { \
_force_has_format_string(__VA_ARGS__); \
} \
} while (0)

#define ERR_QUOTE(str) #str

Expand All @@ -109,48 +116,50 @@ void _force_has_format_string(const char *format, ...) {
* In order to do that (particularly, printing the conditional that failed),
* this can't just wrap RETURN_ERROR().
*/
#define RETURN_ERROR_IF(cond, err, ...) \
if (cond) { \
RAWLOG(3, "%s:%d: ERROR!: check %s failed, returning %s", \
__FILE__, __LINE__, ERR_QUOTE(cond), ERR_QUOTE(ERROR(err))); \
_FORCE_HAS_FORMAT_STRING(__VA_ARGS__); \
RAWLOG(3, ": " __VA_ARGS__); \
RAWLOG(3, "\n"); \
return ERROR(err); \
}
#define RETURN_ERROR_IF(cond, err, ...) \
do { \
if (cond) { \
RAWLOG(3, "%s:%d: ERROR!: check %s failed, returning %s", \
__FILE__, __LINE__, ERR_QUOTE(cond), ERR_QUOTE(ERROR(err))); \
_FORCE_HAS_FORMAT_STRING(__VA_ARGS__); \
RAWLOG(3, ": " __VA_ARGS__); \
RAWLOG(3, "\n"); \
return ERROR(err); \
} \
} while (0)

/**
* Unconditionally return the specified error.
*
* In debug modes, prints additional information.
*/
#define RETURN_ERROR(err, ...) \
do { \
RAWLOG(3, "%s:%d: ERROR!: unconditional check failed, returning %s", \
__FILE__, __LINE__, ERR_QUOTE(ERROR(err))); \
_FORCE_HAS_FORMAT_STRING(__VA_ARGS__); \
RAWLOG(3, ": " __VA_ARGS__); \
RAWLOG(3, "\n"); \
return ERROR(err); \
} while(0);
#define RETURN_ERROR(err, ...) \
do { \
RAWLOG(3, "%s:%d: ERROR!: unconditional check failed, returning %s", \
__FILE__, __LINE__, ERR_QUOTE(ERROR(err))); \
_FORCE_HAS_FORMAT_STRING(__VA_ARGS__); \
RAWLOG(3, ": " __VA_ARGS__); \
RAWLOG(3, "\n"); \
return ERROR(err); \
} while(0)

/**
* If the provided expression evaluates to an error code, returns that error code.
*
* In debug modes, prints additional information.
*/
#define FORWARD_IF_ERROR(err, ...) \
do { \
size_t const err_code = (err); \
if (ERR_isError(err_code)) { \
RAWLOG(3, "%s:%d: ERROR!: forwarding error in %s: %s", \
__FILE__, __LINE__, ERR_QUOTE(err), ERR_getErrorName(err_code)); \
_FORCE_HAS_FORMAT_STRING(__VA_ARGS__); \
RAWLOG(3, ": " __VA_ARGS__); \
RAWLOG(3, "\n"); \
return err_code; \
} \
} while(0);
#define FORWARD_IF_ERROR(err, ...) \
do { \
size_t const err_code = (err); \
if (ERR_isError(err_code)) { \
RAWLOG(3, "%s:%d: ERROR!: forwarding error in %s: %s", \
__FILE__, __LINE__, ERR_QUOTE(err), ERR_getErrorName(err_code)); \
_FORCE_HAS_FORMAT_STRING(__VA_ARGS__); \
RAWLOG(3, ": " __VA_ARGS__); \
RAWLOG(3, "\n"); \
return err_code; \
} \
} while(0)

#if defined (__cplusplus)
}
Expand Down
6 changes: 3 additions & 3 deletions lib/common/zstd_internal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ static void ZSTD_copy8(void* dst, const void* src) {
ZSTD_memcpy(dst, src, 8);
#endif
}
#define COPY8(d,s) { ZSTD_copy8(d,s); d+=8; s+=8; }
#define COPY8(d,s) do { ZSTD_copy8(d,s); d+=8; s+=8; } while (0)

/* Need to use memmove here since the literal buffer can now be located within
the dst buffer. In circumstances where the op "catches up" to where the
Expand All @@ -198,7 +198,7 @@ static void ZSTD_copy16(void* dst, const void* src) {
ZSTD_memcpy(dst, copy16_buf, 16);
#endif
}
#define COPY16(d,s) { ZSTD_copy16(d,s); d+=16; s+=16; }
#define COPY16(d,s) do { ZSTD_copy16(d,s); d+=16; s+=16; } while (0)

#define WILDCOPY_OVERLENGTH 32
#define WILDCOPY_VECLEN 16
Expand Down Expand Up @@ -227,7 +227,7 @@ void ZSTD_wildcopy(void* dst, const void* src, ptrdiff_t length, ZSTD_overlap_e
if (ovtype == ZSTD_overlap_src_before_dst && diff < WILDCOPY_VECLEN) {
/* Handle short offset copies. */
do {
COPY8(op, ip)
COPY8(op, ip);
} while (op < oend);
} else {
assert(diff >= WILDCOPY_VECLEN || diff <= -WILDCOPY_VECLEN);
Expand Down
20 changes: 11 additions & 9 deletions lib/compress/zstd_compress.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -650,10 +650,11 @@ static size_t ZSTD_cParam_clampBounds(ZSTD_cParameter cParam, int* value)
return 0;
}

#define BOUNDCHECK(cParam, val) { \
RETURN_ERROR_IF(!ZSTD_cParam_withinBounds(cParam,val), \
parameter_outOfBound, "Param out of bounds"); \
}
#define BOUNDCHECK(cParam, val) \
do { \
RETURN_ERROR_IF(!ZSTD_cParam_withinBounds(cParam,val), \
parameter_outOfBound, "Param out of bounds"); \
} while (0)


static int ZSTD_isUpdateAuthorized(ZSTD_cParameter param)
Expand Down Expand Up @@ -1392,11 +1393,12 @@ size_t ZSTD_checkCParams(ZSTD_compressionParameters cParams)
static ZSTD_compressionParameters
ZSTD_clampCParams(ZSTD_compressionParameters cParams)
{
# define CLAMP_TYPE(cParam, val, type) { \
ZSTD_bounds const bounds = ZSTD_cParam_getBounds(cParam); \
if ((int)val<bounds.lowerBound) val=(type)bounds.lowerBound; \
else if ((int)val>bounds.upperBound) val=(type)bounds.upperBound; \
}
# define CLAMP_TYPE(cParam, val, type) \
do { \
ZSTD_bounds const bounds = ZSTD_cParam_getBounds(cParam); \
if ((int)val<bounds.lowerBound) val=(type)bounds.lowerBound; \
else if ((int)val>bounds.upperBound) val=(type)bounds.upperBound; \
} while (0)
# define CLAMP(cParam, val) CLAMP_TYPE(cParam, val, unsigned)
CLAMP(ZSTD_c_windowLog, cParams.windowLog);
CLAMP(ZSTD_c_chainLog, cParams.chainLog);
Expand Down
4 changes: 2 additions & 2 deletions lib/compress/zstd_double_fast.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -356,8 +356,8 @@ size_t ZSTD_compressBlock_doubleFast_dictMatchState_generic(
if (ms->prefetchCDictTables) {
size_t const hashTableBytes = (((size_t)1) << dictCParams->hashLog) * sizeof(U32);
size_t const chainTableBytes = (((size_t)1) << dictCParams->chainLog) * sizeof(U32);
PREFETCH_AREA(dictHashLong, hashTableBytes)
PREFETCH_AREA(dictHashSmall, chainTableBytes)
PREFETCH_AREA(dictHashLong, hashTableBytes);
PREFETCH_AREA(dictHashSmall, chainTableBytes);
}

/* init */
Expand Down
2 changes: 1 addition & 1 deletion lib/compress/zstd_fast.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -508,7 +508,7 @@ size_t ZSTD_compressBlock_fast_dictMatchState_generic(

if (ms->prefetchCDictTables) {
size_t const hashTableBytes = (((size_t)1) << dictCParams->hashLog) * sizeof(U32);
PREFETCH_AREA(dictHashTable, hashTableBytes)
PREFETCH_AREA(dictHashTable, hashTableBytes);
}

/* init */
Expand Down
4 changes: 2 additions & 2 deletions lib/compress/zstd_opt.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1182,7 +1182,7 @@ ZSTD_compressBlock_opt_generic(ZSTD_matchState_t* ms,
for (cur = 1; cur <= last_pos; cur++) {
const BYTE* const inr = ip + cur;
assert(cur < ZSTD_OPT_NUM);
DEBUGLOG(7, "cPos:%zi==rPos:%u", inr-istart, cur)
DEBUGLOG(7, "cPos:%zi==rPos:%u", inr-istart, cur);

/* Fix current position with one literal if cheaper */
{ U32 const litlen = (opt[cur-1].mlen == 0) ? opt[cur-1].litlen + 1 : 1;
Expand Down Expand Up @@ -1331,7 +1331,7 @@ ZSTD_compressBlock_opt_generic(ZSTD_matchState_t* ms,
}

/* save sequences */
DEBUGLOG(6, "sending selected sequences into seqStore")
DEBUGLOG(6, "sending selected sequences into seqStore");
{ U32 storePos;
for (storePos=storeStart; storePos <= storeEnd; storePos++) {
U32 const llen = opt[storePos].litlen;
Expand Down
Loading

0 comments on commit 8193250

Please sign in to comment.