Hi Eric,

Normally a patch to the list (or a PR to the github freeipmi-mirror) would be 
fine.  However, the hostlist library is actually copied in from another project.

Talking to the author, the project it came from is not really maintained 
anymore and a newer hostlist here:

https://github.com/flux-framework/flux-core/tree/master/src/common/libhostlist

is the longer term solution.

Sorry the code tripped you up, but perhaps it's best to leave it as is for the 
time being.

Al

________________________________
From: [email protected] 
<[email protected]> on behalf of Eric Herman 
<[email protected]>
Sent: Friday, August 22, 2025 5:49 AM
To: [email protected] <[email protected]>
Subject: assert around an assignment

Hello freeipmi developers,

Looking at common/miscutil/hostlist.c I was rather surprised to see code of the 
form:

assert((i->magic = 0x1));

At a glance, my first thought was that it was an error.

Upon looking closer, I see that the 'int magic' members of the structs are 
guarded by "#ifndef NDEBUG" and came to understand two things.
First, the assert() is being used to turn the assignment into a no-op unless 
debug.
Second, the value of "0x1" instead of "0x0" is chosen to avoid assert failure.

While this works, I found it jarring and I expect other developers may also 
"get tripped up" by it.

Would you be open to a contribution which would make the conditional assignment 
more explicit?

Perhaps something simply along the lines of:

#define hostlist_magic_set(s) assert(((s)->magic = HOSTLIST_MAGIC))
#define hostlist_magic_unset(s) assert(((s)->magic = 0x1))

If so, are there contribution instructions or other contribution guidelines I 
should follow?
Or should I simply send a patch to this list?

Cheers,
-Eric

Reply via email to