Re: [PATCH] Define GNULIB_NAMESPACE in unittests/string_view-selftests.c

Joel Brobecker
2018-05-04T17:20:30Z

> > What worries me is that I don't see what's preventing us from hitting
> > that issue outside of the unittests code? We know we can adjust our
> > own classes, but this problem occured with a system class, so we had
> > no choice but to use GNULIB_NAMESPACE. I worry that the transition
> > from no GNULIB_NAMESPACE to using GNULIB_NAMESPACE in a given unit
> > will leave some C system calls that should normally be covered by
> > gnulib silently now reverting to the system (buggy) version.
>
> Actually, gnulib seems to poison the function it replaces, so we won't be
> able to use it by mistake. Let's say I add a "close" call:
>
> CXX unittests/string_view-selftests.o
> /home/emaisin/src/binutils-gdb/gdb/unittests/string_view-selftests.c: In function ‘void selftests::string_view::run_tests()’:
> /home/emaisin/src/binutils-gdb/gdb/unittests/string_view-selftests.c:169:12: error: call to ‘close’ declared with attribute warning: The symbol ::close refers to the system function. Use gnulib::close instead. [-Werror]
> close (0);
> ^
>
> And since we can enable GNULIB_NAMESPACE per compilation unit, we can
> do it progressively. Once we are done, we can remove all those
> GNULIB_NAMESPACE defines and have a single one in common-defs.h, just
> before including gnulib.

Aha! This completely reassures me. Thanks!

--
Joel