Martin Sebor wrote:
Andrew Black wrote:
Greetings all.
Attached is a patch that aims to resolve a couple minor compile issues
with the exec utility.
The two patches should probably go in separately since they are
completely unrelated.
Patches split and attached
The first concern is the current link behavior, where the standard
library is linked into all executables. While this is normally
desirable, it is considered undesirable for the exec utility, which is
designed without the use of the standard library. The tactic I chose
to use in this patch was to filter the library out of the $LDFLAGS
variable, but I wonder if a better approach would be alter the
makefile.common file so that the library isn't included by default on
the link line.
The ideal approach that we agreed on some time ago was to build
the exec utility as a C program. But yours seems like a good enough
workaround in the meantime (I assume the patch does in fact work
around an actual problem and isn't just cosmetic).
This is a somewhat cosmetic change, but it does allow the exec utility
to run if the library were to build incorrectly such that it crashes
during initialization.
exec_unlink.diff Changelog:
* GNUmakefile.bin (LDFLAGS.exec): Define LDFLAGS variant, filtering
out the stdcxx library
(exec): use LDFLAGS.exec rather than LDFLAGS
The second concern is a compile failure when using the Compaq
compiler. In particular, this compiler fails on code that was
implemented as a workaround for STDCXX-291. I feel that it is
(slightly) more efficient to use the older code than the STDCXX-291
workaround code, so I chose to make that the default path, but it
would also be possible to make the alternate code path specific to the
Compaq compiler (via the __DECCXX macro)
I was going to look into it but you beat me to it. The problem
is that the caller is a C++ function and so the type of the local
function pointer variable is one to a extern "C++" function. We
can't initialize it with a "C" function. What we should do here
is declare a typedef for handle_alrm in the same extern "C" block
as the handler itself and then use the typedef to declare the
local function pointer.
I still feel that it is more efficient to assign the signal handler
reference to the element in the sigaction structure rather than using
memcpy, but I also agree that the use of the typedef is cleaner than
using the #ifndef logic.
compaqfix.diff Changelog:
* exec.cpp (alarm_handler) [!_WIN32 && !_WIN64]: Define typedef
with signature matching that of handle_alrm.
(wait_for_child) [!_WIN32 && !_WIN64]: Use alarm_handler typedef
for type of local variable storing reference to handle_alrm.
--Andrew Black
Index: etc/config/GNUmakefile.bin
===================================================================
--- etc/config/GNUmakefile.bin (revision 472548)
+++ etc/config/GNUmakefile.bin (working copy)
@@ -45,6 +45,9 @@
LDFLAGS += $(CPPFLAGS)
endif # ($(CXX_REPOSITORY),)
+# Don't want to link exec utility with stdlib, so create our own LDFLAGS var
+LDFLAGS.exec = $(filter-out -l$(LIBBASE),$(LDFLAGS))
+
##############################################################################
# TARGETS
##############################################################################
@@ -55,8 +58,8 @@
# link the run utility
exec: runall.o cmdopt.o output.o util.o exec.o display.o
- @echo "$(LD) $^ -o $@ $(LDFLAGS) $(LDLIBS)" >> $(LOGFILE)
- $(LD) $^ -o $@ $(LDFLAGS) $(LDLIBS) $(TEEOPTS)
+ @echo "$(LD) $^ -o $@ $(LDFLAGS.exec) $(LDLIBS)" >> $(LOGFILE)
+ $(LD) $^ -o $@ $(LDFLAGS.exec) $(LDLIBS) $(TEEOPTS)
# link the localedef utility
localedef: localedef.o aliases.o charmap.o codecvt.o collate.o ctype.o \
Index: exec.cpp
===================================================================
--- exec.cpp (revision 472548)
+++ exec.cpp (working copy)
@@ -360,6 +360,8 @@
alarm_timeout = 1;
}
+typedef void (*alarm_handler)(int);
+
#ifdef __cplusplus
}
#endif
@@ -421,7 +423,7 @@
/* avoid extern "C"/"C++" mismatch due to an HP aCC 6 bug
(see STDCXX-291)
*/
- void (*phandler)(int) = handle_alrm;
+ alarm_handler phandler = handle_alrm;
memcpy (&act.sa_handler, &phandler, sizeof act.sa_handler);
sigaction (SIGALRM, &act, 0);
