Re: Patch to improve linker output format selection

[Ian Lance Taylor <ian@zembu.com>]

   Date: Fri, 16 Jul 1999 15:36:15 +0100
   From: Nick Clifton <nickc@cygnus.com>

       1. If an output format has been specified on the command line, then
	  use that.

       2. Otherwise if an endianness has been specified on the command
	  line then:

	     a. If the default output format has the required endianness
		then use that.

	     b. Otherwise attempt to locate the target format which is the
		closest match to the default output format, but which has
		the required endian characteristic.

       3. Otherwise attempt to discover the format of the first of the
	  linker's input files and set the output format to the same.

       4. Otherwise if that search fails use the default output format.

I always worry about code like this where we try to outsmart the user.
I can see why we want to do it.  But consider the case of a
user-written linker script: the user has explicitly used the
OUTPUT_FORMAT command in the linker script.  With your patch, the
linker will now be clever and instead use the format of the first
input file.  We are effectively forcing the user to use the -oformat
option in this case.

This is a weird case, so that is not so bad.  But it's not a backward
compatible change, which means we need documentation changes and NEWS
changes.

Actually, the more I look at your patch, the weirder it seems.  In
case 2 you prefer the default output format.  In case 3 you prefer the
first input file.  What is the essential difference between these
cases?  Why shouldn't you always prefer the first input file?  In
fact, doesn't this patch make OUTPUT_FORMAT essentially useless?  It
sets a default, but that default will only be used in highly
exceptional cases.


   + static char *
   + get_first_input_target (s)
   +      lang_statement_union_type * s;

It would be better to write this using FOR_EACH_INPUT_STATEMENT.

   + 	  if (s->input_statement.real == true)

Please don't compare boolean values to true or false.  I know there is
existing code which does it, but I don't like it.  Just write
    if (s->input_statement.real)


   *************** lang_add_output_format (format, big, lit
   *** 4030,4036 ****
	   else if (command_line.endian == ENDIAN_LITTLE
		  && little != NULL)
	   format = little;
   ! 
	   output_target = format;
	 }
     }
   - --- 4171,4181 ----
	   else if (command_line.endian == ENDIAN_LITTLE
		  && little != NULL)
	   format = little;
   !       else if (from_script)
   ! 	{
   ! 	  default_target = (char *) format;
   ! 	  format = NULL;
   ! 	}
	   output_target = format;
	 }
     }

This looks wrong.  We are changing the meaning of OUTPUT_FORMAT in a
linker script, which I guess is OK.  But why should we only change it
when only one format is specified?  Basically I don't understand why
it is correct to have the ``else'' in the lines you added.

Ian