Disabling top level fixincludes

Disabling top level fixincludes

[Bruce Korb]

Hi guys,

C.F.: http://gcc.gnu.org/ml/gcc-patches/2004-09/msg00934.html

Looks good to me.  My solution was just a no-op script when nothing
was wanted.  The main thing is to avoid doing a lot of work when
nothing needs doing.  :)  It looks like you've glued it into
a more general purpose construct.


C.F.: http://gcc.gnu.org/ml/gcc-patches/2004-09/msg01393.html

Fixing portability is a good thing.  I would try to reduce ifdef-ed
code though with things like:

  #ifndef SIGKILL
  #  define SIGKILL SIGTERM
  #endif

and then simply replace all:

  fcntl (stdout_pair.write_fd, F_DUPFD, STDOUT_FILENO);

type invocations with:

  dup2 (stdout_pair.write_fd, STDOUT_FILENO);

because the latter should always work.  Better still, macro-ize
the silly thing:

  #define DUP_FD( _ofd, _nfd ) \
    dup2 ( (_ofd), (_nfd) )

and if some platform needs the fcntl, another #define gets added
and the code remains unchanged.

Cheers - Bruce

Re: Disabling top level fixincludes

[Zack Weinberg]

Bruce Korb <bkorb@veritas.com> writes:

...
> and then simply replace all:
>
>   fcntl (stdout_pair.write_fd, F_DUPFD, STDOUT_FILENO);
>
> type invocations with:
>
>   dup2 (stdout_pair.write_fd, STDOUT_FILENO);
>
> because the latter should always work.  Better still, macro-ize
> the silly thing:
>
>   #define DUP_FD( _ofd, _nfd ) \
>     dup2 ( (_ofd), (_nfd) )
>
> and if some platform needs the fcntl, another #define gets added
> and the code remains unchanged.

Even better, leave the dup2 in the code alone, and - only if someone
complains - put

#if !defined HAVE_DUP2 && defined HAVE_FCNTL_DUPFD
#define dup2(ofd, nfd) fcntl(ofd, F_DUPFD, nfd)
#endif

in an appropriate header, plus appropriate configure goo.  (This is
preferable to DUP_FD because people already know what dup2() does,
whereas for DUP_FD we have to go look up the macro definition.)

zw

Re: Disabling top level fixincludes

[Bruce Korb]

Zack Weinberg wrote:

> Even better, leave the dup2 in the code alone, and - only if someone
> complains - put
> 
> #if !defined HAVE_DUP2 && defined HAVE_FCNTL_DUPFD
> #define dup2(ofd, nfd) fcntl(ofd, F_DUPFD, nfd)
> #endif
> 
> in an appropriate header, plus appropriate configure goo.  (This is
> preferable to DUP_FD because people already know what dup2() does,
> whereas for DUP_FD we have to go look up the macro definition.)

That's fine, just as long as the ifdef-s are removed from the
code.  Eyeball parsing of alternating code text is just that much
more bother in trying to understand what you are reading.

:)  Thanks - Bruce

Re: Disabling top level fixincludes

[Kaveh R. Ghazi]

 > Even better, leave the dup2 in the code alone, and - only if someone
 > complains - put
 > 
 > #if !defined HAVE_DUP2 && defined HAVE_FCNTL_DUPFD
 > #define dup2(ofd, nfd) fcntl(ofd, F_DUPFD, nfd)
 > #endif
 > 
 > in an appropriate header, plus appropriate configure goo.

Even better, put all the "goo" in a libiberty dup2 replacement.

There's also a dup2 replacement in collect2.c that uses a different
backup mechanism than fcntl, this could also be merged and zapped.

		--Kaveh
--
Kaveh R. Ghazi			ghazi@caip.rutgers.edu

Re: Disabling top level fixincludes

[Aaron W. LaFramboise]

[-- Attachment #1: Type: text/plain, Size: 217 bytes --]

Bruce Korb wrote:

> C.F.: http://gcc.gnu.org/ml/gcc-patches/2004-09/msg01393.html

Attached is a revised patch.

Once this is applied, and PR17991 is resolved, fixincludes will build on
MinGW.

Aaron W. LaFramboise


[-- Attachment #2: gcc-mainline-20041015-fixincl.patch --]
[-- Type: text/plain, Size: 4003 bytes --]

2004-10-15  Aaron W. LaFramboise <aaronavay62@aaronwl.com>

	* fixincl.c (SIGCHLD): Remove definition.
	(initialize): Remove SIGIOT and SIGPIPE checks.
	(create_file): Fix mkdir() for Win32.
	(internal_fix): Use dup2() instead of fcntl().
	* fixlib.h (SIGQUIT): Define if undefined.
	(SIGIOT): Same.
	(SIGPIPE): Same.
	(SIGALRM): Same.
	(SIGKILL): Same.
	* procopen.c (chain_open): Use dup2() instead of fcntl().

Index: gcc/fixincludes/fixincl.c
===================================================================
RCS file: /cvsroot/gcc/gcc/fixincludes/fixincl.c,v
retrieving revision 1.2
diff -c -3 -p -r1.2 fixincl.c
*** gcc/fixincludes/fixincl.c	15 Oct 2004 07:58:38 -0000	1.2
--- gcc/fixincludes/fixincl.c	15 Oct 2004 23:12:15 -0000
*************** Boston, MA 02111-1307, USA.  */
*** 30,38 ****
  #define  BAD_ADDR ((void*)-1)
  #endif
  
- #if ! defined( SIGCHLD ) && defined( SIGCLD )
- #  define SIGCHLD SIGCLD
- #endif
  #ifndef SEPARATE_FIX_PROC
  #include "server.h"
  #endif
--- 30,35 ----
*************** initialize ( int argc, char** argv )
*** 291,302 ****
  # endif
  
    signal (SIGQUIT, SIG_IGN);
- #ifdef SIGIOT
    signal (SIGIOT,  SIG_IGN);
- #endif
- #ifdef SIGPIPE
    signal (SIGPIPE, SIG_IGN);
- #endif
    signal (SIGALRM, SIG_IGN);
    signal (SIGTERM, SIG_IGN);
  }
--- 288,295 ----
*************** create_file (void)
*** 552,558 ****
--- 545,555 ----
            *pz_dir = NUL;
            if (stat (fname, &stbf) < 0)
              {
+ #ifdef _WIN32
+               mkdir (fname);
+ #else
                mkdir (fname, S_IFDIR | S_DIRALL);
+ #endif
              }
  
            *pz_dir = '/';
*************** internal_fix (int read_fd, tFixDesc* p_f
*** 835,842 ****
     *  Make the fd passed in the stdin, and the write end of
     *  the new pipe become the stdout.
     */
!   fcntl (fd[1], F_DUPFD, STDOUT_FILENO);
!   fcntl (read_fd, F_DUPFD, STDIN_FILENO);
  
    apply_fix (p_fixd, pz_curr_file);
    exit (0);
--- 832,839 ----
     *  Make the fd passed in the stdin, and the write end of
     *  the new pipe become the stdout.
     */
!   dup2 (fd[1], STDOUT_FILENO);
!   dup2 (read_fd, STDIN_FILENO);
  
    apply_fix (p_fixd, pz_curr_file);
    exit (0);
Index: gcc/fixincludes/fixlib.h
===================================================================
RCS file: /cvsroot/gcc/gcc/fixincludes/fixlib.h,v
retrieving revision 1.2
diff -c -3 -p -r1.2 fixlib.h
*** gcc/fixincludes/fixlib.h	15 Oct 2004 07:58:38 -0000	1.2
--- gcc/fixincludes/fixlib.h	15 Oct 2004 23:12:15 -0000
*************** Boston, MA 02111-1307, USA.  */
*** 40,45 ****
--- 40,65 ----
  # define STDOUT_FILENO  1
  #endif
  
+ #if ! defined( SIGCHLD ) && defined( SIGCLD )
+ #  define SIGCHLD SIGCLD
+ #endif
+ 
+ #ifndef SIGQUIT
+ #define SIGQUIT SIGTERM
+ #endif
+ #ifndef SIGIOT
+ #define SIGIOT SIGTERM
+ #endif
+ #ifndef SIGPIPE
+ #define SIGPIPE SIGTERM
+ #endif
+ #ifndef SIGALRM
+ #define SIGALRM SIGTERM
+ #endif
+ #ifndef SIGKILL
+ #define SIGKILL SIGTERM
+ #endif
+ 
  typedef int t_success;
  
  #define FAILURE         (-1)
Index: gcc/fixincludes/procopen.c
===================================================================
RCS file: /cvsroot/gcc/gcc/fixincludes/procopen.c,v
retrieving revision 1.1
diff -c -3 -p -r1.1 procopen.c
*** gcc/fixincludes/procopen.c	31 Aug 2004 09:26:05 -0000	1.1
--- gcc/fixincludes/procopen.c	15 Oct 2004 23:12:15 -0000
*************** chain_open (int stdin_fd, tCC** pp_args,
*** 155,162 ****
     *  Make the fd passed in the stdin, and the write end of
     *  the new pipe become the stdout.
     */
!   fcntl (stdout_pair.write_fd, F_DUPFD, STDOUT_FILENO);
!   fcntl (stdin_fd, F_DUPFD, STDIN_FILENO);
  
    if (*pp_args == (char *) NULL)
      *pp_args = pz_cmd;
--- 155,162 ----
     *  Make the fd passed in the stdin, and the write end of
     *  the new pipe become the stdout.
     */
!   dup2 (stdout_pair.write_fd, STDOUT_FILENO);
!   dup2 (stdin_fd, STDIN_FILENO);
  
    if (*pp_args == (char *) NULL)
      *pp_args = pz_cmd;

Re: Disabling top level fixincludes

[Bruce Korb]

"Aaron W. LaFramboise" wrote:
> 
> Bruce Korb wrote:
> 
> > C.F.: http://gcc.gnu.org/ml/gcc-patches/2004-09/msg01393.html
> 
> Attached is a revised patch.
> 
> Once this is applied, and PR17991 is resolved, fixincludes will build on
> MinGW.

Excellent.  Thanks. - Bruce

Re: Disabling top level fixincludes

[Aaron W. LaFramboise]

[-- Attachment #1: Type: text/plain, Size: 977 bytes --]

Bruce Korb wrote:

> "Aaron W. LaFramboise" wrote:

>>Bruce Korb wrote:

>>>C.F.: http://gcc.gnu.org/ml/gcc-patches/2004-09/msg01393.html
>>
>>Attached is a revised patch.
>>
>>Once this is applied, and PR17991 is resolved, fixincludes will build on
>>MinGW.
> 
> Excellent.  Thanks. - Bruce

Now that two-process fixincludes is fixed and builds, I see that I
missed one Windows-compatibility bit: Windows doesn't have pathconf().
The logic in the code that uses pathconf() does not actually seem
related to filename truncation, as the comments suggest, but rather
detecting the case where more than one dot in a filename is not allowed.
 I suspect that this code only works for DJGPP, and it isn't needed on
Windows anyway, so I think its best just to disable it if pathconf()
doesn't exist.

By the way, while I have a copyright assignment, I don't have CVS
access, so if you or someone else could commit these two patches for me,
I would be grateful.

Aaron W. LaFramboise


[-- Attachment #2: gcc-mainline-20041019-fixincl.patch --]
[-- Type: text/plain, Size: 1013 bytes --]

2004-10-19  Aaron W. LaFramboise <aaronavay62@aaronwl.com>

	* fixfixes.c (main): Check for _PC_NAME_MAX.

Index: gcc/fixincludes/fixfixes.c
===================================================================
RCS file: /cvsroot/gcc/gcc/fixincludes/fixfixes.c,v
retrieving revision 1.2
diff -c -3 -p -r1.2 fixfixes.c
*** gcc/fixincludes/fixfixes.c	15 Oct 2004 07:58:38 -0000	1.2
--- gcc/fixincludes/fixfixes.c	19 Oct 2004 19:26:37 -0000
*************** main( int argc, char** argv )
*** 778,787 ****
--- 778,789 ----
       doesn't allow more than one dot in the trunk of a file name.  */
    pz_tmp_base = basename( pz_tmptmp );
    pz_tmp_dot = strchr( pz_tmp_base, '.' );
+ #ifdef _PC_NAME_MAX
    if (pathconf( pz_tmptmp, _PC_NAME_MAX ) <= 12	/* is this DOS or Windows9X? */
        && pz_tmp_dot != (char*)NULL)
      strcpy (pz_tmp_dot+1, "X"); /* nuke the original extension */
    else
+ #endif /* _PC_NAME_MAX */
      strcat (pz_tmptmp, ".X");
    if (freopen (pz_tmptmp, "w", stdout) != stdout)
      {