Mailing-List: contact stdcxx-dev-help@incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: stdcxx-dev@incubator.apache.org
Received-SPF: pass (herse.apache.org: local policy)
Message-ID: <4552488E.1040206@roguewave.com>
Date: Wed, 08 Nov 2006 14:13:50 -0700
From: Martin Sebor <sebor@roguewave.com>
Organization: Rogue Wave Software
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.13) Gecko/20060417
MIME-Version: 1.0
To: stdcxx-dev@incubator.apache.org
Subject: Re: [PATCH]Assorted cleanup
References: <455220C6.2040006@roguewave.com>
In-Reply-To: <455220C6.2040006@roguewave.com>
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit

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.

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

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

Martin

> 
> --Andrew Black
> 
> Changelog:
>     * GNUmakefile.bin (LDFLAGS.exec): Define LDFLAGS variant, filtering 
> out the stdcxx library
>       (exec): use LDFLAGS.exec rather than LDFLAGS
>     * exec.cpp (wait_for_child): Use workaround for STDCXX-291 only with 
> HP aCC (causes failure with Compaq CXX)
> 
> 
> ------------------------------------------------------------------------
> 
> 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: util/exec.cpp
> ===================================================================
> --- util/exec.cpp	(revision 472548)
> +++ util/exec.cpp	(working copy)
> @@ -421,8 +421,12 @@
>      /* avoid extern "C"/"C++" mismatch due to an HP aCC 6 bug
>         (see STDCXX-291)
>      */
> +#ifndef __HP_aCC
> +    act.sa_handler = handle_alrm;
> +#else
>      void (*phandler)(int) = handle_alrm;
>      memcpy (&act.sa_handler, &phandler, sizeof act.sa_handler);
> +#endif
>  
>      sigaction (SIGALRM, &act, 0);
>