This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: gcc/collect2.c patch



  In message <199903241613.QAA29064@out4.ibm.net>you write:
  > I've attached my reworked version. Here is a ChangeLog:
  > 
  > * gcc/config/i386/xm-djgpp.h (COLLECT2_INIT_SYS_DEPENDANT): 
  > New macro.
  > * gcc/collect2.c (main): Use it.
  >   (pexecute_pid): New variable. Holds return value from call to pexecute.
  >   (collect2_execute): Rework to use pexecute instead of fork.
  >   (collect2_wait): Use pwait() instead of wait().
Thanks.  Better, but still some problems.

  > + /* Perform system dependant initialization, if neccessary.  */
  > + #if defined (COLLECT2_INIT_SYS_DEPENDANT)
  > + COLLECT2_INIT_SYS_DEPENDANT;
  > + #endif
Put the comment inside the #ifdef, then indent the comment and invocation of
the macro.  I also think COLLECT_HOST_INITIALIZATION would be a better name.
[ie, indicate clearly that it is host specific init stuff. ]

  > !       /* Duplicate the stdout and stderr file handles
  > !          so they can be restored later.  */
  > !       stdout_save = dup (STDOUT_FILENO);
  > !       if (stdout_save == -1)
  > !         fatal_perror ("redirecting stdout: %s", redir);
  > !       stderr_save = dup (STDERR_FILENO);
  > !       if (stderr_save == -1)
  > !         fatal_perror ("redirecting stdout: %s", redir);
I don't think you can depend on STDOUT_FILENO or STDERR_FILENO being defined.
You may want to put default definitions for them into system.h like this:

#ifndef STDIN_FILENO
# define STDIN_FILENO   0
#endif
etc...



  > !       /* Redirect stdout & stderr to our response file.  */
  > !       dup2(redir_handle, STDOUT_FILENO);
  > !       dup2(redir_handle, STDERR_FILENO);
Don't forget the space between the function name and the open paren for the
arguments.

  > !   pexecute_pid = pexecute(argv[0], argv, argv[0], NULL,
  > !                      &errmsg_fmt, &errmsg_arg,
  > !                      (PEXECUTE_FIRST | PEXECUTE_LAST | PEXECUTE_SEARCH));
Similarly.  You forgot the space.  The arguments are also indented incorrectly.
It should look like this:

  pexecute_pid = pexecute (argv[0], argv, argv[0], NULL,
			   &errmsg_fmt, &errmsg_arg,
			   (PEXECUTE_FIRST | PEXECUTE_LAST | PEXECUTE_SEARCH));

Also, use tabs, not 8 spaces.

  > + /* System dependant initialization for collect2
  > +    to tell system() to act like Unix.  */
  > + #define COLLECT2_INIT_SYS_DEPENDANT \
  > + do {__system_flags |= (__system_allow_multiple_cmds | __system_emulate_ch
  > dir); } while (0)
This long line needs to be wrapped.


I've fixed these problems this time.  But in the future I will send back patches
which do not conform to the GNU coding standards.  Please review the GNU coding
standards before you submit additional changes.

Thanks,

jeff


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]