ku1ik / stderred

stderr in red
MIT License
996 stars 115 forks source link

Rename init() to avoid name collision with running program #65

Closed chrisjbillington closed 3 years ago

chrisjbillington commented 5 years ago

I've been debugging a bizarre issue recently with the panel program tint2, that I have traced to my use of stderred. When I start the tint2 panel, mouse clicks were being ignored half the time, and it turned out there were two copies of the panel running that were interfering with each other. Upon digging into the source code, it turns out that tint2 has a function called init(), which was being called twice, when it should only be called once, leading to the incorrect behaviour. One of these calls was even occurring before the program's main() method! This hinted that it was not a bug in tint2 and led me to remember what I had in my LD_PRELOAD.

Upon testing, I've determined that this problem doesn't occur when I am not setting LD_PRELOAD to use stderred. It appears to be a name collision with the constructor of stderred, which is also called init(). I'm not sure under what conditions it is triggered (I tried to make a minimal C program to demonstrate the issue, but was not able to), but basically the init() function from tint2 was being called instead of stderred's constructor, resulting in tint2 initialising itself twice and not working correctly.

This pull request renames init() to stderred_init(). With this change, having stderred in LD_PRELOAD doesn't affect the program tint2 anymore.

I am assuming that the fact that init() is a constructor makes it special and that the other functions are not at risk of name collisions (other than the ones actually intended to replace libc calls), but if this assumption is not correct then perhaps other functions should be name-munged as well

chrisjbillington commented 5 years ago

Any thoughts on this?

chrisjbillington commented 5 years ago

Done - it's now a single commit on top of current master matching your commit message convention, with the renamed function now being init_stderred to match reset_stderred, and by not renaming the struct member in the test code there are fewer changes.

chrisjbillington commented 5 years ago

Any love for merging this?

dset0x commented 5 years ago

@chrisjbillington could you check if #72 fixes this for you? Should be the cleaner solution.

cehoffman commented 3 years ago

Should be resolved from #72

chrisjbillington commented 3 years ago

Thank you!