From stdcxx-dev-return-2339-apmail-incubator-stdcxx-dev-archive=incubator.apache.org@incubator.apache.org Thu Nov 09 21:53:35 2006 Return-Path: Delivered-To: apmail-incubator-stdcxx-dev-archive@www.apache.org Received: (qmail 30850 invoked from network); 9 Nov 2006 21:53:35 -0000 Received: from hermes.apache.org (HELO mail.apache.org) (140.211.11.2) by minotaur.apache.org with SMTP; 9 Nov 2006 21:53:35 -0000 Received: (qmail 50539 invoked by uid 500); 9 Nov 2006 21:53:46 -0000 Delivered-To: apmail-incubator-stdcxx-dev-archive@incubator.apache.org Received: (qmail 50481 invoked by uid 500); 9 Nov 2006 21:53:46 -0000 Mailing-List: contact stdcxx-dev-help@incubator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: stdcxx-dev@incubator.apache.org Delivered-To: mailing list stdcxx-dev@incubator.apache.org Received: (qmail 50470 invoked by uid 99); 9 Nov 2006 21:53:46 -0000 Received: from herse.apache.org (HELO herse.apache.org) (140.211.11.133) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 09 Nov 2006 13:53:46 -0800 X-ASF-Spam-Status: No, hits=0.0 required=10.0 tests= X-Spam-Check-By: apache.org Received-SPF: pass (herse.apache.org: local policy) Received: from [208.30.140.160] (HELO moroha.quovadx.com) (208.30.140.160) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 09 Nov 2006 13:53:32 -0800 Received: from [10.70.3.48] ([10.70.3.48]) by moroha.quovadx.com (8.13.6/8.13.6) with ESMTP id kA9Lqwte017896 for ; Thu, 9 Nov 2006 21:52:58 GMT Message-ID: <4553A332.3000901@roguewave.com> Date: Thu, 09 Nov 2006 14:52:50 -0700 From: Andrew Black User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.7) Gecko/20060910 SeaMonkey/1.0.5 MIME-Version: 1.0 To: stdcxx-dev@incubator.apache.org Subject: Re: [PATCH]Assorted cleanup References: <455220C6.2040006@roguewave.com> <4552488E.1040206@roguewave.com> In-Reply-To: <4552488E.1040206@roguewave.com> Content-Type: multipart/mixed; boundary="------------050209090107020903020505" X-Virus-Checked: Checked by ClamAV on apache.org --------------050209090107020903020505 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit 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 --------------050209090107020903020505 Content-Type: text/x-patch; name="exec_unlink.diff" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="exec_unlink.diff" 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 \ --------------050209090107020903020505 Content-Type: text/x-patch; name="compaqfix.diff" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="compaqfix.diff" 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); --------------050209090107020903020505--