jiridanek commented on a change in pull request #1267:
URL: https://github.com/apache/qpid-dispatch/pull/1267#discussion_r655430987
##########
File path: src/server.c
##########
@@ -1738,7 +1738,7 @@ void qd_connector_decref(qd_connector_t* connector)
}
}
-__attribute__((noinline)) // permit replacement by dummy implementation in
unit_tests
+__attribute__((weak)) // permit replacement by dummy implementation in
unit_tests
Review comment:
I'd love to do something about it, but there isn't really an existing
universal solution (in the form of a library) that I'd be happy with. And I
never spent the time.
Doing linking substitution like this actually is not so bad. It is even
recommended (in it's original version, with a shared library) in
http://blog.wingman-sw.com/linker-substitution-in-c-limitations-and-workarounds.
When switching from shared library to a static one, I could've either kept
building the so in addition, just for use in tests (that is a good idea, AFAIK,
for the future), or change the tests to use the other technique mentioned in
the article (the pointer substitution one). Or use weak symbols, which produces
the smallest diff.
I'm actually happy that we can get rid of `noinline`, that was bugging me.
Clang by default wants to inline one of the functions on -O2 and we have to
prevent that because of our tests. I feel better about having weak symbols
around, but that might be just me. Maybe there is some harm in them that I am
just not seeing. (Besides the obvious: you might redefine it when you don't
mean to.) Noinline is btw necessary with the runtime injection APIs. Or compile
for tests extra and turn off optimizations altogether.
Regarding the function pointer substitution technique from the blog. It is
the first time I saw it and it is actually a very good solution, also it uses
standard C without any platform magic (either linker trickery or ELF file
manipulation). It looks really simple to understand, but it's actually quite
clever. I wouldn't 've come up with it myself.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
