Message ID | 20171124103207.19233-1-arkadiusz.hiler@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Quoting Arkadiusz Hiler (2017-11-24 10:32:07) > write_stderr() and __write_stderr() are defined behind ifdef on > HAVE_LIBUNWIND, but do no depend on the lib in any way. Hmm, we keep getting caught out by large ifdefs. Although I guess conditionally enabled code itself is less-than-welcome, we should probably aim to make whole C files be conditionally compiled, exporting no-op interfaces when not enabled. Just a thought. -Chris
On 24/11/2017 10:32, Arkadiusz Hiler wrote: > write_stderr() and __write_stderr() are defined behind ifdef on > HAVE_LIBUNWIND, but do no depend on the lib in any way. > > fatal_sig_handler() uses those helpers unconditionally. > > This patch just moves the code couple of lines up, so the helpers are > always available and do not break build on systems without libunwind. > > Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> > Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com> > --- > lib/igt_core.c | 20 ++++++++++---------- > 1 file changed, 10 insertions(+), 10 deletions(-) > > diff --git a/lib/igt_core.c b/lib/igt_core.c > index 4fe48c97..6ce83bec 100644 > --- a/lib/igt_core.c > +++ b/lib/igt_core.c > @@ -1231,6 +1231,16 @@ static bool run_under_gdb(void) > strncmp(basename(buf), "gdb", 3) == 0); > } > > +static void __write_stderr(const char *str, size_t len) > +{ > + igt_ignore_warn(write(STDERR_FILENO, str, len)); > +} > + > +static void write_stderr(const char *str) > +{ > + __write_stderr(str, strlen(str)); > +} > + > #ifdef HAVE_LIBUNWIND > #define UNW_LOCAL_ONLY > #include <libunwind.h> > @@ -1407,16 +1417,6 @@ xprintf(const char *fmt, ...) > va_end(ap); > } > > -static void __write_stderr(const char *str, size_t len) > -{ > - igt_ignore_warn(write(STDERR_FILENO, str, len)); > -} > - > -static void write_stderr(const char *str) > -{ > - __write_stderr(str, strlen(str)); > -} > - > static void print_backtrace_sig_safe(void) > { > unw_cursor_t cursor; > Oopsie, I did not spot that #ifdef block. Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Regards, Tvrtko
On Fri, Nov 24, 2017 at 10:51:38AM +0000, Chris Wilson wrote: > Quoting Arkadiusz Hiler (2017-11-24 10:32:07) > > write_stderr() and __write_stderr() are defined behind ifdef on > > HAVE_LIBUNWIND, but do no depend on the lib in any way. > > Hmm, we keep getting caught out by large ifdefs. Although I guess > conditionally enabled code itself is less-than-welcome, we should > probably aim to make whole C files be conditionally compiled, exporting > no-op interfaces when not enabled. Just a thought. > -Chris Sounds about right, and... unwinding the libunwind mess would be a good way to start. Any volunteers? I may be able to give it a shot next week...
diff --git a/lib/igt_core.c b/lib/igt_core.c index 4fe48c97..6ce83bec 100644 --- a/lib/igt_core.c +++ b/lib/igt_core.c @@ -1231,6 +1231,16 @@ static bool run_under_gdb(void) strncmp(basename(buf), "gdb", 3) == 0); } +static void __write_stderr(const char *str, size_t len) +{ + igt_ignore_warn(write(STDERR_FILENO, str, len)); +} + +static void write_stderr(const char *str) +{ + __write_stderr(str, strlen(str)); +} + #ifdef HAVE_LIBUNWIND #define UNW_LOCAL_ONLY #include <libunwind.h> @@ -1407,16 +1417,6 @@ xprintf(const char *fmt, ...) va_end(ap); } -static void __write_stderr(const char *str, size_t len) -{ - igt_ignore_warn(write(STDERR_FILENO, str, len)); -} - -static void write_stderr(const char *str) -{ - __write_stderr(str, strlen(str)); -} - static void print_backtrace_sig_safe(void) { unw_cursor_t cursor;
write_stderr() and __write_stderr() are defined behind ifdef on HAVE_LIBUNWIND, but do no depend on the lib in any way. fatal_sig_handler() uses those helpers unconditionally. This patch just moves the code couple of lines up, so the helpers are always available and do not break build on systems without libunwind. Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com> --- lib/igt_core.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)