Andrew Black wrote:
[...]
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

Okay. Please do try to capitalize/terminate your sentences in
the ChangeLog. ChangeLogs deserve the same attention to detail
as the code changes they document.


[...]
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.

Directly assigning the pointer probably is going to be more
efficient in most cases (some optimizers, such as gcc, manage
to eliminate the memcpy call and replace it with the equivalent
all over of the plain assignment, but others, such as Sun C++,
do not). But the point of the memcpy call here is to avoid the
incompatibility and to work around the HP aCC bug. We don't
need to be that concerned with efficiency in this code, so I
wouldn't lose sleep over it if I were you :)


compaqfix.diff Changelog:
    * exec.cpp (alarm_handler) [!_WIN32 && !_WIN64]:

There is no need for the &&. .. part here or in code as the _WIN32
macro is guaranteed to be defined. Please stop using it -- it only
makes the text harder to read. Other than that, if this compiles
with aCC 6 and Compaq C++ 7 go ahead and commit it.

Thanks
Martin

 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);

Reply via email to