sddf icon indicating copy to clipboard operation
sddf copied to clipboard

`util.h` namespace pollution

Open alexandermbrown opened this issue 1 year ago • 2 comments

sddf/util.h is automatically included in user's code through files like sddf/serial/queue.h. This means whenever a user uses sDDF their namespace is polluted with our definitions of ARRAY_SIZE, MIN, MAX, unlikely, assert, etc. I don't think this is acceptable as a user may unknowingly use sDDF's version of assert (and similar) which is often defined in the user's project or in the standard library. Additionally, it commonly results in macro redefined warnings.

In other libraries, this is solved by either

  • adding a namespace prefix to these definitions (i.e., sddf_assert and SDDF_MIN etc.)
  • only including util.h in C files so it doesn't get exposed to the public API (this would require changing project structure and build system which I assume we don't want to do)
  • put #undefs at the end of sddf/serial/queue.h to stop it leaking to user code (not a fan of this as its quite messy)

A bandaid fix is to add #ifndef before all definitions however this doesn't solve the problem if a user includes their definitions after sDDF's.

TODO

  • [x] Add ifndef to MIN, MAX and ROUND_UP (https://github.com/au-ts/sddf/pull/163)
  • [ ] Change assert to sddf_assert
  • [ ] Remove likely and unlikely?

alexandermbrown avatar Jul 16 '24 00:07 alexandermbrown

The proper solution is involving a portable libc but until we have that the best thing to do to avoid unintentional clashes it to rename it to sddf_assert.

Ivan-Velickovic avatar Jul 16 '24 00:07 Ivan-Velickovic

I don't know why unlikely and likely exist. They should be removed since they're not used in the code base, all the other macros like ARRAY_SIZE etc can just be wrapped in #ifndef.

Ivan-Velickovic avatar Jul 16 '24 00:07 Ivan-Velickovic