[RFC] Move ``length'' from struct main_type to struct type

[RFC] Move ``length'' from struct main_type to struct type

[Kevin Buettner]

A while back, I introduced a change to dwarf2read.c in
read_tag_pointer_type() which was supposed to create a pointer type
variant of a (potentially) different length:

  if (TYPE_LENGTH (type) != byte_size || addr_class != DW_ADDR_none)
    {
      if (ADDRESS_CLASS_TYPE_FLAGS_P ())
	{
	  int type_flags;

	  type_flags = ADDRESS_CLASS_TYPE_FLAGS (byte_size, addr_class);
	  gdb_assert ((type_flags & ~TYPE_FLAG_ADDRESS_CLASS_ALL) == 0);
	  type = make_type_with_address_space (type, type_flags);
	}
    ...
    }

  TYPE_LENGTH (type) = byte_size;

However, this code doesn't work correctly.  As it stands now, the type
length is shared between all type variants (which differ only in
qualifiers).  This means that as soon as the above TYPE_LENGTH
assignment is performed, all type variants end up getting the same
length.  I should note that when I initially developed this code, I
did so on a branch which did not yet implement this sharing via struct
main_type.

The patch below corrects this problem by moving the length field from
struct main_type to struct type.  I am not entirely happy with this
approach, but the other approaches I've considered are even less
palatable.

E.g, another approach that I considered would be to create a new type
which has a different main_type that varies only in the length field. 
The problem with this is that the names end up being the same, and it
seems to me that there will be problems with searching and finding the
right type when the user specifies it by name.

Certainly, if anyone can think of a better approach, I'd be happy to
hear about it.

Those of you reviewing this patch should carefully consider the
comment in replace_type().  I don't think the problem that I mention
there will actually arise since replace_type() is only called by symbol
readers which don't know how to (and indeed can't) create variants of
different sizes, so the situation described in the replace_type() comment
should never (at the moment anyway) arise.

Comments?  (I'll wait at least a week before checking this one in.)

	* gdbtypes.h (struct main_type): Move ``length'' field from here...
	(struct type): ...to here.
	(TYPE_LENGTH): Adjust to reflect different location of ``length''
	field.
	* gdbtypes.c (make_qualified_type): Set length on newly created type.
	(replace_type): Set length on all type variants for a given type.

Index: gdbtypes.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbtypes.c,v
retrieving revision 1.69
diff -u -p -r1.69 gdbtypes.c
--- gdbtypes.c	17 Jan 2003 19:12:18 -0000	1.69
+++ gdbtypes.c	29 Jan 2003 22:14:40 -0000
@@ -469,6 +469,9 @@ make_qualified_type (struct type *type, 
   /* Now set the instance flags and return the new type.  */
   TYPE_INSTANCE_FLAGS (ntype) = new_flags;
 
+  /* Set length of new type to that of the original type.  */
+  TYPE_LENGTH (ntype) = TYPE_LENGTH (type);
+
   return ntype;
 }
 
@@ -556,9 +559,23 @@ make_cv_type (int cnst, int voltl, struc
 void
 replace_type (struct type *ntype, struct type *type)
 {
-  struct type *cv_chain, *as_chain, *ptr, *ref;
+  struct type *chain;
 
   *TYPE_MAIN_TYPE (ntype) = *TYPE_MAIN_TYPE (type);
+
+  /* The type length is not a part of the main type.  Update it for each
+     type on the variant chain.  Note that this is not correct for variants
+     on the chain which are supposed to have a length different than other
+     variants on the chain, but that's one of the drawbacks of constructing
+     types this way.  We *could* only change the lengths which are the same
+     as NTYPE's original length, but that doesn't really help because we
+     don't know what the new lengths should be for the types that don't
+     match.  */
+  chain = ntype;
+  do {
+    TYPE_LENGTH (ntype) = TYPE_LENGTH (type);
+    chain = TYPE_CHAIN (chain);
+  } while (ntype != chain);
 
   /* Assert that the two types have equivalent instance qualifiers.
      This should be true for at least all of our debug readers.  */
Index: gdbtypes.h
===================================================================
RCS file: /cvs/src/src/gdb/gdbtypes.h,v
retrieving revision 1.42
diff -u -p -r1.42 gdbtypes.h
--- gdbtypes.h	19 Jan 2003 04:06:45 -0000	1.42
+++ gdbtypes.h	29 Jan 2003 22:14:41 -0000
@@ -297,32 +297,6 @@ struct main_type
 
   char *tag_name;
 
-  /* Length of storage for a value of this type.  This is what
-     sizeof(type) would return; use it for address arithmetic,
-     memory reads and writes, etc.  This size includes padding.  For
-     example, an i386 extended-precision floating point value really
-     only occupies ten bytes, but most ABI's declare its size to be
-     12 bytes, to preserve alignment.  A `struct type' representing
-     such a floating-point type would have a `length' value of 12,
-     even though the last two bytes are unused.
-
-     There's a bit of a host/target mess here, if you're concerned
-     about machines whose bytes aren't eight bits long, or who don't
-     have byte-addressed memory.  Various places pass this to memcpy
-     and such, meaning it must be in units of host bytes.  Various
-     other places expect they can calculate addresses by adding it
-     and such, meaning it must be in units of target bytes.  For
-     some DSP targets, in which HOST_CHAR_BIT will (presumably) be 8
-     and TARGET_CHAR_BIT will be (say) 32, this is a problem.
-
-     One fix would be to make this field in bits (requiring that it
-     always be a multiple of HOST_CHAR_BIT and TARGET_CHAR_BIT) ---
-     the other choice would be to make it consistently in units of
-     HOST_CHAR_BIT.  However, this would still fail to address
-     machines based on a ternary or decimal representation.  */
-  
-  unsigned length;
-
   /* FIXME, these should probably be restricted to a Fortran-specific
      field in some fashion.  */
 #define BOUND_CANNOT_BE_DETERMINED   5
@@ -489,15 +463,42 @@ struct type
   struct type *reference_type;
 
   /* Variant chain.  This points to a type that differs from this one only
-     in qualifiers.  Currently, the possible qualifiers are const, volatile,
-     code-space, and data-space.  The variants are linked in a circular
-     ring and share MAIN_TYPE.  */
+     in qualifiers and length.  Currently, the possible qualifiers are
+     const, volatile, code-space, data-space, and address class.  The
+     length may differ only when one of the address class flags are set.
+     The variants are linked in a circular ring and share MAIN_TYPE.  */
   struct type *chain;
 
   /* Flags specific to this instance of the type, indicating where
      on the ring we are.  */
   int instance_flags;
 
+  /* Length of storage for a value of this type.  This is what
+     sizeof(type) would return; use it for address arithmetic,
+     memory reads and writes, etc.  This size includes padding.  For
+     example, an i386 extended-precision floating point value really
+     only occupies ten bytes, but most ABI's declare its size to be
+     12 bytes, to preserve alignment.  A `struct type' representing
+     such a floating-point type would have a `length' value of 12,
+     even though the last two bytes are unused.
+
+     There's a bit of a host/target mess here, if you're concerned
+     about machines whose bytes aren't eight bits long, or who don't
+     have byte-addressed memory.  Various places pass this to memcpy
+     and such, meaning it must be in units of host bytes.  Various
+     other places expect they can calculate addresses by adding it
+     and such, meaning it must be in units of target bytes.  For
+     some DSP targets, in which HOST_CHAR_BIT will (presumably) be 8
+     and TARGET_CHAR_BIT will be (say) 32, this is a problem.
+
+     One fix would be to make this field in bits (requiring that it
+     always be a multiple of HOST_CHAR_BIT and TARGET_CHAR_BIT) ---
+     the other choice would be to make it consistently in units of
+     HOST_CHAR_BIT.  However, this would still fail to address
+     machines based on a ternary or decimal representation.  */
+  
+  unsigned length;
+
   /* Core type, shared by a group of qualified types.  */
   struct main_type *main_type;
 };
@@ -758,7 +759,7 @@ extern void allocate_cplus_struct_type (
    But check_typedef does set the TYPE_LENGTH of the TYPEDEF type,
    so you only have to call check_typedef once.  Since allocate_value
    calls check_typedef, TYPE_LENGTH (VALUE_TYPE (X)) is safe.  */
-#define TYPE_LENGTH(thistype) TYPE_MAIN_TYPE(thistype)->length
+#define TYPE_LENGTH(thistype) (thistype)->length
 #define TYPE_OBJFILE(thistype) TYPE_MAIN_TYPE(thistype)->objfile
 #define TYPE_FLAGS(thistype) TYPE_MAIN_TYPE(thistype)->flags
 /* Note that TYPE_CODE can be TYPE_CODE_TYPEDEF, so if you want the real

Re: [RFC] Move ``length'' from struct main_type to struct type

[Daniel Jacobowitz]

On Wed, Jan 29, 2003 at 03:48:29PM -0700, Kevin Buettner wrote:
> A while back, I introduced a change to dwarf2read.c in
> read_tag_pointer_type() which was supposed to create a pointer type
> variant of a (potentially) different length:
> 
>   if (TYPE_LENGTH (type) != byte_size || addr_class != DW_ADDR_none)
>     {
>       if (ADDRESS_CLASS_TYPE_FLAGS_P ())
> 	{
> 	  int type_flags;
> 
> 	  type_flags = ADDRESS_CLASS_TYPE_FLAGS (byte_size, addr_class);
> 	  gdb_assert ((type_flags & ~TYPE_FLAG_ADDRESS_CLASS_ALL) == 0);
> 	  type = make_type_with_address_space (type, type_flags);
> 	}
>     ...
>     }
> 
>   TYPE_LENGTH (type) = byte_size;
> 
> However, this code doesn't work correctly.  As it stands now, the type
> length is shared between all type variants (which differ only in
> qualifiers).  This means that as soon as the above TYPE_LENGTH
> assignment is performed, all type variants end up getting the same
> length.  I should note that when I initially developed this code, I
> did so on a branch which did not yet implement this sharing via struct
> main_type.
> 
> The patch below corrects this problem by moving the length field from
> struct main_type to struct type.  I am not entirely happy with this
> approach, but the other approaches I've considered are even less
> palatable.
> 
> E.g, another approach that I considered would be to create a new type
> which has a different main_type that varies only in the length field. 
> The problem with this is that the names end up being the same, and it
> seems to me that there will be problems with searching and finding the
> right type when the user specifies it by name.
> 
> Certainly, if anyone can think of a better approach, I'd be happy to
> hear about it.

It won't actually create a problem if you give them different main
types; the main type is purely internal to the type system, and is
never searched directly.

That said, while I'm not really happy with your approach either, I
think it is more correct and less fragile in general.

> Those of you reviewing this patch should carefully consider the
> comment in replace_type().  I don't think the problem that I mention
> there will actually arise since replace_type() is only called by symbol
> readers which don't know how to (and indeed can't) create variants of
> different sizes, so the situation described in the replace_type() comment
> should never (at the moment anyway) arise.
> 
> Comments?  (I'll wait at least a week before checking this one in.)

My only comment is that I'd rather you check in replace_type that there
are no types on the variant ring with different space qualifiers, and
internal_error if there are.  How's that sound?


-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFC] Move ``length'' from struct main_type to struct type

[Kevin Buettner]

On Jan 29,  6:15pm, Daniel Jacobowitz wrote:

> My only comment is that I'd rather you check in replace_type that there
> are no types on the variant ring with different space qualifiers, and
> internal_error if there are.  How's that sound?

I like it.  I've appended a revised patch below.

	* gdbtypes.h (struct main_type): Move ``length'' field from here...
	(struct type): ...to here.
	(TYPE_LENGTH): Adjust to reflect different location of ``length''
	field.
	* gdbtypes.c (make_qualified_type): Set length on newly created type.
	(replace_type): Set length on all type variants for a given type.

Index: gdbtypes.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbtypes.c,v
retrieving revision 1.69
diff -u -p -r1.69 gdbtypes.c
--- gdbtypes.c	17 Jan 2003 19:12:18 -0000	1.69
+++ gdbtypes.c	30 Jan 2003 00:51:04 -0000
@@ -469,6 +469,9 @@ make_qualified_type (struct type *type, 
   /* Now set the instance flags and return the new type.  */
   TYPE_INSTANCE_FLAGS (ntype) = new_flags;
 
+  /* Set length of new type to that of the original type.  */
+  TYPE_LENGTH (ntype) = TYPE_LENGTH (type);
+
   return ntype;
 }
 
@@ -556,9 +559,25 @@ make_cv_type (int cnst, int voltl, struc
 void
 replace_type (struct type *ntype, struct type *type)
 {
-  struct type *cv_chain, *as_chain, *ptr, *ref;
+  struct type *chain;
 
   *TYPE_MAIN_TYPE (ntype) = *TYPE_MAIN_TYPE (type);
+
+  /* The type length is not a part of the main type.  Update it for each
+     type on the variant chain.  */
+  chain = ntype;
+  do {
+    /* Assert that this element of the chain has no address-class bits
+       set in its flags.  Such type variants might have type lengths
+       which are supposed to be different from the non-address-class
+       variants.  This assertion shouldn't ever be triggered because
+       symbol readers which do construct address-class variants don't
+       call replace_type().  */
+    gdb_assert (TYPE_ADDRESS_CLASS_ALL (chain) == 0);
+
+    TYPE_LENGTH (ntype) = TYPE_LENGTH (type);
+    chain = TYPE_CHAIN (chain);
+  } while (ntype != chain);
 
   /* Assert that the two types have equivalent instance qualifiers.
      This should be true for at least all of our debug readers.  */
Index: gdbtypes.h
===================================================================
RCS file: /cvs/src/src/gdb/gdbtypes.h,v
retrieving revision 1.42
diff -u -p -r1.42 gdbtypes.h
--- gdbtypes.h	19 Jan 2003 04:06:45 -0000	1.42
+++ gdbtypes.h	30 Jan 2003 00:51:05 -0000
@@ -297,32 +297,6 @@ struct main_type
 
   char *tag_name;
 
-  /* Length of storage for a value of this type.  This is what
-     sizeof(type) would return; use it for address arithmetic,
-     memory reads and writes, etc.  This size includes padding.  For
-     example, an i386 extended-precision floating point value really
-     only occupies ten bytes, but most ABI's declare its size to be
-     12 bytes, to preserve alignment.  A `struct type' representing
-     such a floating-point type would have a `length' value of 12,
-     even though the last two bytes are unused.
-
-     There's a bit of a host/target mess here, if you're concerned
-     about machines whose bytes aren't eight bits long, or who don't
-     have byte-addressed memory.  Various places pass this to memcpy
-     and such, meaning it must be in units of host bytes.  Various
-     other places expect they can calculate addresses by adding it
-     and such, meaning it must be in units of target bytes.  For
-     some DSP targets, in which HOST_CHAR_BIT will (presumably) be 8
-     and TARGET_CHAR_BIT will be (say) 32, this is a problem.
-
-     One fix would be to make this field in bits (requiring that it
-     always be a multiple of HOST_CHAR_BIT and TARGET_CHAR_BIT) ---
-     the other choice would be to make it consistently in units of
-     HOST_CHAR_BIT.  However, this would still fail to address
-     machines based on a ternary or decimal representation.  */
-  
-  unsigned length;
-
   /* FIXME, these should probably be restricted to a Fortran-specific
      field in some fashion.  */
 #define BOUND_CANNOT_BE_DETERMINED   5
@@ -489,15 +463,42 @@ struct type
   struct type *reference_type;
 
   /* Variant chain.  This points to a type that differs from this one only
-     in qualifiers.  Currently, the possible qualifiers are const, volatile,
-     code-space, and data-space.  The variants are linked in a circular
-     ring and share MAIN_TYPE.  */
+     in qualifiers and length.  Currently, the possible qualifiers are
+     const, volatile, code-space, data-space, and address class.  The
+     length may differ only when one of the address class flags are set.
+     The variants are linked in a circular ring and share MAIN_TYPE.  */
   struct type *chain;
 
   /* Flags specific to this instance of the type, indicating where
      on the ring we are.  */
   int instance_flags;
 
+  /* Length of storage for a value of this type.  This is what
+     sizeof(type) would return; use it for address arithmetic,
+     memory reads and writes, etc.  This size includes padding.  For
+     example, an i386 extended-precision floating point value really
+     only occupies ten bytes, but most ABI's declare its size to be
+     12 bytes, to preserve alignment.  A `struct type' representing
+     such a floating-point type would have a `length' value of 12,
+     even though the last two bytes are unused.
+
+     There's a bit of a host/target mess here, if you're concerned
+     about machines whose bytes aren't eight bits long, or who don't
+     have byte-addressed memory.  Various places pass this to memcpy
+     and such, meaning it must be in units of host bytes.  Various
+     other places expect they can calculate addresses by adding it
+     and such, meaning it must be in units of target bytes.  For
+     some DSP targets, in which HOST_CHAR_BIT will (presumably) be 8
+     and TARGET_CHAR_BIT will be (say) 32, this is a problem.
+
+     One fix would be to make this field in bits (requiring that it
+     always be a multiple of HOST_CHAR_BIT and TARGET_CHAR_BIT) ---
+     the other choice would be to make it consistently in units of
+     HOST_CHAR_BIT.  However, this would still fail to address
+     machines based on a ternary or decimal representation.  */
+  
+  unsigned length;
+
   /* Core type, shared by a group of qualified types.  */
   struct main_type *main_type;
 };
@@ -758,7 +759,7 @@ extern void allocate_cplus_struct_type (
    But check_typedef does set the TYPE_LENGTH of the TYPEDEF type,
    so you only have to call check_typedef once.  Since allocate_value
    calls check_typedef, TYPE_LENGTH (VALUE_TYPE (X)) is safe.  */
-#define TYPE_LENGTH(thistype) TYPE_MAIN_TYPE(thistype)->length
+#define TYPE_LENGTH(thistype) (thistype)->length
 #define TYPE_OBJFILE(thistype) TYPE_MAIN_TYPE(thistype)->objfile
 #define TYPE_FLAGS(thistype) TYPE_MAIN_TYPE(thistype)->flags
 /* Note that TYPE_CODE can be TYPE_CODE_TYPEDEF, so if you want the real

Re: [RFC] Move ``length'' from struct main_type to struct type

[Kevin Buettner]

On Jan 29,  3:48pm, Kevin Buettner wrote:

> 	* gdbtypes.h (struct main_type): Move ``length'' field from here...
> 	(struct type): ...to here.
> 	(TYPE_LENGTH): Adjust to reflect different location of ``length''
> 	field.
> 	* gdbtypes.c (make_qualified_type): Set length on newly created type.
> 	(replace_type): Set length on all type variants for a given type.

Committed.

RFC: Use program_transform_name correctly

[Jim Blandy]

It seems as if some Makefiles aren't properly using
program_transform_name.

The same kind of weirdness corrected(?) in the patch below appears in
gdb/Makefile.in, so I'm not at all sure I'm not misunderstanding
what's going on.  If folks agree that the change below would be
correct, then I'll put together a larger patch that fixes the ones I
can find elsewhere, too.

(Not sure why this hasn't come up before; Daniel J.'s recent posts on
the topic seem to be about setting program_transform_name, not on how
to use it.)

2003-10-07  Jim Blandy  <jimb@redhat.com>

	* Makefile.in (RUNTEST_FOR_TARGET): Pass the transformation to set
	properly.

Index: gdb/testsuite//Makefile.in
===================================================================
RCS file: /cvs/cvsfiles/devo/gdb/testsuite/Makefile.in,v
retrieving revision 1.66
diff -c -r1.66 Makefile.in
*** gdb/testsuite//Makefile.in	9 Sep 2003 21:03:53 -0000	1.66
--- gdb/testsuite//Makefile.in	7 Oct 2003 21:33:22 -0000
***************
*** 53,59 ****
      if [ "$(host_canonical)" = "$(target_canonical)" ]; then \
        echo runtest; \
      else \
!       t='$(program_transform_name)'; echo runtest | sed -e '' $$t; \
      fi; \
    fi`
  
--- 53,59 ----
      if [ "$(host_canonical)" = "$(target_canonical)" ]; then \
        echo runtest; \
      else \
!       t='$(program_transform_name)'; echo runtest | sed -e '$$t'; \
      fi; \
    fi`
  

Re: RFC: Use program_transform_name correctly

[Daniel Jacobowitz]

On Tue, Oct 07, 2003 at 04:41:35PM -0500, Jim Blandy wrote:
> 
> It seems as if some Makefiles aren't properly using
> program_transform_name.
> 
> The same kind of weirdness corrected(?) in the patch below appears in
> gdb/Makefile.in, so I'm not at all sure I'm not misunderstanding
> what's going on.  If folks agree that the change below would be
> correct, then I'll put together a larger patch that fixes the ones I
> can find elsewhere, too.
> 
> (Not sure why this hasn't come up before; Daniel J.'s recent posts on
> the topic seem to be about setting program_transform_name, not on how
> to use it.)

That is _bizarre_.  Does it even give you anything but a sed error now?

> 2003-10-07  Jim Blandy  <jimb@redhat.com>
> 
> 	* Makefile.in (RUNTEST_FOR_TARGET): Pass the transformation to set
> 	properly.
> 
> Index: gdb/testsuite//Makefile.in
> ===================================================================
> RCS file: /cvs/cvsfiles/devo/gdb/testsuite/Makefile.in,v
> retrieving revision 1.66
> diff -c -r1.66 Makefile.in
> *** gdb/testsuite//Makefile.in	9 Sep 2003 21:03:53 -0000	1.66
> --- gdb/testsuite//Makefile.in	7 Oct 2003 21:33:22 -0000
> ***************
> *** 53,59 ****
>       if [ "$(host_canonical)" = "$(target_canonical)" ]; then \
>         echo runtest; \
>       else \
> !       t='$(program_transform_name)'; echo runtest | sed -e '' $$t; \
>       fi; \
>     fi`
>   
> --- 53,59 ----
>       if [ "$(host_canonical)" = "$(target_canonical)" ]; then \
>         echo runtest; \
>       else \
> !       t='$(program_transform_name)'; echo runtest | sed -e '$$t'; \
>       fi; \
>     fi`

That's certainly what it's supposed to look like.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: RFC: Use program_transform_name correctly

[Theodore A. Roth]

On Tue, 7 Oct 2003, Daniel Jacobowitz wrote:

> On Tue, Oct 07, 2003 at 04:41:35PM -0500, Jim Blandy wrote:
> >
> > It seems as if some Makefiles aren't properly using
> > program_transform_name.
> >
> > The same kind of weirdness corrected(?) in the patch below appears in
> > gdb/Makefile.in, so I'm not at all sure I'm not misunderstanding
> > what's going on.  If folks agree that the change below would be
> > correct, then I'll put together a larger patch that fixes the ones I
> > can find elsewhere, too.
> >
> > (Not sure why this hasn't come up before; Daniel J.'s recent posts on
> > the topic seem to be about setting program_transform_name, not on how
> > to use it.)
>
> That is _bizarre_.  Does it even give you anything but a sed error now?
>
> > 2003-10-07  Jim Blandy  <jimb@redhat.com>
> >
> > 	* Makefile.in (RUNTEST_FOR_TARGET): Pass the transformation to set
> > 	properly.
> >
> > Index: gdb/testsuite//Makefile.in
> > ===================================================================
> > RCS file: /cvs/cvsfiles/devo/gdb/testsuite/Makefile.in,v
> > retrieving revision 1.66
> > diff -c -r1.66 Makefile.in
> > *** gdb/testsuite//Makefile.in	9 Sep 2003 21:03:53 -0000	1.66
> > --- gdb/testsuite//Makefile.in	7 Oct 2003 21:33:22 -0000
> > ***************
> > *** 53,59 ****
> >       if [ "$(host_canonical)" = "$(target_canonical)" ]; then \
> >         echo runtest; \
> >       else \
> > !       t='$(program_transform_name)'; echo runtest | sed -e '' $$t; \
> >       fi; \
> >     fi`
> >
> > --- 53,59 ----
> >       if [ "$(host_canonical)" = "$(target_canonical)" ]; then \
> >         echo runtest; \
> >       else \
> > !       t='$(program_transform_name)'; echo runtest | sed -e '$$t'; \
> >       fi; \
> >     fi`
>
> That's certainly what it's supposed to look like.
>

You sure that the expression is right? I think the single quotes will
hide the expansion of $t in the shell. I ran a simple test on my
system with this make file fragment:

roth@knuth:/tmp$ cat Makefile
program_transform_name = s/^/avr-/

all:
        t='$(program_transform_name)'; echo runtest | sed -e '$$t'
        t='$(program_transform_name)'; echo runtest | sed -e "$$t"
roth@knuth:/tmp$ make
t='s/^/avr-/'; echo runtest | sed -e '$t'
runtest
t='s/^/avr-/'; echo runtest | sed -e "$t"
avr-runtest


Ted Roth

Re: RFC: Use program_transform_name correctly

[Felix Lee]

Jim Blandy <jimb@redhat.com>:
> !       t='$(program_transform_name)'; echo runtest | sed -e '' $$t; \

it looks to me like this is written for a program_transform_name
that's a concatenated list of -e options.  the -e '' will keep
sed from complaining if program_transform_name is null.

I remember at some point in the past, sometimes
program_transform_name was a semicolon-separated list of sed
commands, sometimes it was a list of sed -e options.  Don't know
if it's consistent now.
--

Re: RFC: Use program_transform_name correctly

[Daniel Jacobowitz]

On Tue, Oct 07, 2003 at 03:10:59PM -0700, Theodore A. Roth wrote:
> 
> 
> On Tue, 7 Oct 2003, Daniel Jacobowitz wrote:
> 
> > On Tue, Oct 07, 2003 at 04:41:35PM -0500, Jim Blandy wrote:
> > >
> > > It seems as if some Makefiles aren't properly using
> > > program_transform_name.
> > >
> > > The same kind of weirdness corrected(?) in the patch below appears in
> > > gdb/Makefile.in, so I'm not at all sure I'm not misunderstanding
> > > what's going on.  If folks agree that the change below would be
> > > correct, then I'll put together a larger patch that fixes the ones I
> > > can find elsewhere, too.
> > >
> > > (Not sure why this hasn't come up before; Daniel J.'s recent posts on
> > > the topic seem to be about setting program_transform_name, not on how
> > > to use it.)
> >
> > That is _bizarre_.  Does it even give you anything but a sed error now?
> >
> > > 2003-10-07  Jim Blandy  <jimb@redhat.com>
> > >
> > > 	* Makefile.in (RUNTEST_FOR_TARGET): Pass the transformation to set
> > > 	properly.
> > >
> > > Index: gdb/testsuite//Makefile.in
> > > ===================================================================
> > > RCS file: /cvs/cvsfiles/devo/gdb/testsuite/Makefile.in,v
> > > retrieving revision 1.66
> > > diff -c -r1.66 Makefile.in
> > > *** gdb/testsuite//Makefile.in	9 Sep 2003 21:03:53 -0000	1.66
> > > --- gdb/testsuite//Makefile.in	7 Oct 2003 21:33:22 -0000
> > > ***************
> > > *** 53,59 ****
> > >       if [ "$(host_canonical)" = "$(target_canonical)" ]; then \
> > >         echo runtest; \
> > >       else \
> > > !       t='$(program_transform_name)'; echo runtest | sed -e '' $$t; \
> > >       fi; \
> > >     fi`
> > >
> > > --- 53,59 ----
> > >       if [ "$(host_canonical)" = "$(target_canonical)" ]; then \
> > >         echo runtest; \
> > >       else \
> > > !       t='$(program_transform_name)'; echo runtest | sed -e '$$t'; \
> > >       fi; \
> > >     fi`
> >
> > That's certainly what it's supposed to look like.
> >
> 
> You sure that the expression is right? I think the single quotes will
> hide the expansion of $t in the shell. I ran a simple test on my
> system with this make file fragment:
> 
> roth@knuth:/tmp$ cat Makefile
> program_transform_name = s/^/avr-/
> 
> all:
>         t='$(program_transform_name)'; echo runtest | sed -e '$$t'
>         t='$(program_transform_name)'; echo runtest | sed -e "$$t"
> roth@knuth:/tmp$ make
> t='s/^/avr-/'; echo runtest | sed -e '$t'
> runtest
> t='s/^/avr-/'; echo runtest | sed -e "$t"
> avr-runtest

Eh, you're right, this will teach me to answer without looking.  From
gcc/Makefile.in:

       t='$(program_transform_cross_name)'; echo ar | sed -e $$t ; \

That's the idiom we should use here.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: RFC: Use program_transform_name correctly

[Felix Lee]

Daniel Jacobowitz <drow@mvista.com>:
> Eh, you're right, this will teach me to answer without looking.  From
> gcc/Makefile.in:
>        t='$(program_transform_cross_name)'; echo ar | sed -e $$t ; \
> That's the idiom we should use here.

yeah, $$t should be unquoted because configure does the quoting,
so that a transformation that has quotes will work correctly.
however, this is only for "recent" versions of autoconf, where
recent is "some version of autoconf after 2.13 that still says
'generated by 2.13' at the top of configure".

autoconf 2.13 and before will sometimes leave
program_transform_name null, which will cause that sed to fail.

the newer autoconfs will make sure program_transform_name is
's,x,x,' if it would otherwise be null.
--

Re: RFC: Use program_transform_name correctly

[Daniel Jacobowitz]

On Tue, Oct 07, 2003 at 04:59:21PM -0700, Felix Lee wrote:
> Daniel Jacobowitz <drow@mvista.com>:
> > Eh, you're right, this will teach me to answer without looking.  From
> > gcc/Makefile.in:
> >        t='$(program_transform_cross_name)'; echo ar | sed -e $$t ; \
> > That's the idiom we should use here.
> 
> yeah, $$t should be unquoted because configure does the quoting,
> so that a transformation that has quotes will work correctly.
> however, this is only for "recent" versions of autoconf, where
> recent is "some version of autoconf after 2.13 that still says
> 'generated by 2.13' at the top of configure".
> 
> autoconf 2.13 and before will sometimes leave
> program_transform_name null, which will cause that sed to fail.

How horrid.

> the newer autoconfs will make sure program_transform_name is
> 's,x,x,' if it would otherwise be null.

Conveniently this is no longer an issue in the src repository (or in
gcc).  The toplevel configure script now guarantees that
$(program_transform_name) will not be empty.  So $$t it is.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: RFC: Use program_transform_name correctly

[Jim Blandy]

Felix Lee <felix.1@canids.net> writes:

> Jim Blandy <jimb@redhat.com>:
> > !       t='$(program_transform_name)'; echo runtest | sed -e '' $$t; \
> 
> it looks to me like this is written for a program_transform_name
> that's a concatenated list of -e options.  the -e '' will keep
> sed from complaining if program_transform_name is null.
> 
> I remember at some point in the past, sometimes
> program_transform_name was a semicolon-separated list of sed
> commands, sometimes it was a list of sed -e options.  Don't know
> if it's consistent now.

All right --- that at least explains what the intent was.  However,
GNU sed and the POSIX spec now both agree that, if a -e option is
present, any additional arguments are filenames, not scripts.  So this
is definitely broken.

Re: RFC: Use program_transform_name correctly

[Jim Blandy]

Daniel Jacobowitz <drow@mvista.com> writes:

> On Tue, Oct 07, 2003 at 04:41:35PM -0500, Jim Blandy wrote:
> > 
> > It seems as if some Makefiles aren't properly using
> > program_transform_name.
> > 
> > The same kind of weirdness corrected(?) in the patch below appears in
> > gdb/Makefile.in, so I'm not at all sure I'm not misunderstanding
> > what's going on.  If folks agree that the change below would be
> > correct, then I'll put together a larger patch that fixes the ones I
> > can find elsewhere, too.
> > 
> > (Not sure why this hasn't come up before; Daniel J.'s recent posts on
> > the topic seem to be about setting program_transform_name, not on how
> > to use it.)
> 
> That is _bizarre_.  Does it even give you anything but a sed error
> now?

No, it always gives me an error.  But I think this may be the first
time I've ever tried building a cross-debugger in a tree that lacks
its own dejagnu.

Re: RFC: Use program_transform_name correctly

[Jim Blandy]

Daniel Jacobowitz <drow@mvista.com> writes:

> On Tue, Oct 07, 2003 at 04:59:21PM -0700, Felix Lee wrote:
> > Daniel Jacobowitz <drow@mvista.com>:
> > > Eh, you're right, this will teach me to answer without looking.  From
> > > gcc/Makefile.in:
> > >        t='$(program_transform_cross_name)'; echo ar | sed -e $$t ; \
> > > That's the idiom we should use here.
> > 
> > yeah, $$t should be unquoted because configure does the quoting,
> > so that a transformation that has quotes will work correctly.
> > however, this is only for "recent" versions of autoconf, where
> > recent is "some version of autoconf after 2.13 that still says
> > 'generated by 2.13' at the top of configure".
> > 
> > autoconf 2.13 and before will sometimes leave
> > program_transform_name null, which will cause that sed to fail.
> 
> How horrid.
> 
> > the newer autoconfs will make sure program_transform_name is
> > 's,x,x,' if it would otherwise be null.
> 
> Conveniently this is no longer an issue in the src repository (or in
> gcc).  The toplevel configure script now guarantees that
> $(program_transform_name) will not be empty.  So $$t it is.

Okay.  I'd better fix this up before I forget all this.

Re: RFC: Use program_transform_name correctly

[Jim Blandy]

Daniel Jacobowitz <drow@mvista.com> writes:
> Conveniently this is no longer an issue in the src repository (or in
> gcc).  The toplevel configure script now guarantees that
> $(program_transform_name) will not be empty.  So $$t it is.

But doesn't that mean I can just say:

        sed '$(program_transform_name)'

since Make substitution doesn't respect single quotes in shell
commands?

Re: RFC: Use program_transform_name correctly

[Daniel Jacobowitz]

On Wed, Oct 08, 2003 at 02:17:26PM -0500, Jim Blandy wrote:
> 
> Daniel Jacobowitz <drow@mvista.com> writes:
> > Conveniently this is no longer an issue in the src repository (or in
> > gcc).  The toplevel configure script now guarantees that
> > $(program_transform_name) will not be empty.  So $$t it is.
> 
> But doesn't that mean I can just say:
> 
>         sed '$(program_transform_name)'
> 
> since Make substitution doesn't respect single quotes in shell
> commands?

I'm not sure.  This is all complicated by the way that things get
escaped if $(program_transform_name) contains a ' or a \.  And sed
scripts often do contain backslashes:
  --program-transform-name='s,(gdb),\1-6.0,'

(or maybe that needs more backslashes, I always forget)

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: RFC: Use program_transform_name correctly

[Jim Blandy]

Could someone proofread this boring patch very carefully?

gdb/Makefile.in:
2003-10-08  Jim Blandy  <jimb@redhat.com>

	* Makefile.in (CC_FOR_TARGET, CXX_FOR_TARGET, install-only): Use
	program_transform_name properly.
	* nlm/Makefile.in (CC_FOR_TARGET, NLMCONV_FOR_TARGET): Same.

gdb/gdbserver/Makefile.in:
2003-10-08  Jim Blandy  <jimb@redhat.com>

	* Makefile.in (install-only, uninstall): Use
	program_transform_name properly.

gdb/testsuite/Makefile.in:
2003-10-08  Jim Blandy  <jimb@redhat.com>

	* Makefile.in (RUNTEST_FOR_TARGET): Use program_transform_name
	properly.

Index: gdb/Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/Makefile.in,v
retrieving revision 1.454
diff -c -r1.454 Makefile.in
*** gdb/Makefile.in	8 Oct 2003 02:41:49 -0000	1.454
--- gdb/Makefile.in	8 Oct 2003 19:22:39 -0000
***************
*** 465,471 ****
      if [ "$(host_canonical)" = "$(target_canonical)" ] ; then \
        echo $(CC); \
      else \
!       t='$(program_transform_name)'; echo gcc | sed -e '' $$t; \
      fi; \
    fi`
  
--- 465,471 ----
      if [ "$(host_canonical)" = "$(target_canonical)" ] ; then \
        echo $(CC); \
      else \
!       t='$(program_transform_name)'; echo gcc | sed -e $$t; \
      fi; \
    fi`
  
***************
*** 481,487 ****
      if [ "$(host_canonical)" = "$(target_canonical)" ] ; then \
        echo $(CXX); \
      else \
!       t='$(program_transform_name)'; echo gcc | sed -e '' $$t; \
      fi; \
    fi`
  
--- 481,487 ----
      if [ "$(host_canonical)" = "$(target_canonical)" ] ; then \
        echo $(CXX); \
      else \
!       t='$(program_transform_name)'; echo gcc | sed -e $$t; \
      fi; \
    fi`
  
***************
*** 963,969 ****
  install: all install-only
  install-only: $(CONFIG_INSTALL)
  	transformed_name=`t='$(program_transform_name)'; \
! 			  echo gdb | sed -e "$$t"` ; \
  		if test "x$$transformed_name" = x; then \
  		  transformed_name=gdb ; \
  		else \
--- 963,969 ----
  install: all install-only
  install-only: $(CONFIG_INSTALL)
  	transformed_name=`t='$(program_transform_name)'; \
! 			  echo gdb | sed -e $$t` ; \
  		if test "x$$transformed_name" = x; then \
  		  transformed_name=gdb ; \
  		else \
Index: gdb/gdbserver/Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/Makefile.in,v
retrieving revision 1.24
diff -c -r1.24 Makefile.in
*** gdb/gdbserver/Makefile.in	8 Aug 2003 17:30:36 -0000	1.24
--- gdb/gdbserver/Makefile.in	8 Oct 2003 19:22:40 -0000
***************
*** 149,155 ****
  # install-only is intended to address that need.
  install: all install-only
  install-only: 
! 	n=`echo gdbserver | sed '$(program_transform_name)'`; \
  	if [ x$$n = x ]; then n=gdbserver; else true; fi; \
  	$(SHELL) $(srcdir)/../../mkinstalldirs $(DESTDIR)$(bindir); \
  	$(INSTALL_PROGRAM) gdbserver $(DESTDIR)$(bindir)/$$n; \
--- 149,155 ----
  # install-only is intended to address that need.
  install: all install-only
  install-only: 
! 	n=`t='$(program_transform_name)'; echo gdbserver | sed $$t`; \
  	if [ x$$n = x ]; then n=gdbserver; else true; fi; \
  	$(SHELL) $(srcdir)/../../mkinstalldirs $(DESTDIR)$(bindir); \
  	$(INSTALL_PROGRAM) gdbserver $(DESTDIR)$(bindir)/$$n; \
***************
*** 157,163 ****
  	$(INSTALL_DATA) $(srcdir)/gdbserver.1 $(DESTDIR)$(man1dir)/$$n.1
  
  uninstall: force
! 	n=`echo gdbserver | sed '$(program_transform_name)'`; \
  	if [ x$$n = x ]; then n=gdbserver; else true; fi; \
  	rm -f $(bindir)/$$n $(DESTDIR)$(man1dir)/$$n.1
  
--- 157,163 ----
  	$(INSTALL_DATA) $(srcdir)/gdbserver.1 $(DESTDIR)$(man1dir)/$$n.1
  
  uninstall: force
! 	n=`t='$(program_transform_name)'; echo gdbserver | sed $$t`; \
  	if [ x$$n = x ]; then n=gdbserver; else true; fi; \
  	rm -f $(bindir)/$$n $(DESTDIR)$(man1dir)/$$n.1
  
Index: gdb/nlm/Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/nlm/Makefile.in,v
retrieving revision 1.2
diff -c -r1.2 Makefile.in
*** gdb/nlm/Makefile.in	6 Mar 2001 08:21:46 -0000	1.2
--- gdb/nlm/Makefile.in	8 Oct 2003 19:22:40 -0000
***************
*** 55,68 ****
    if [ -f ../../gcc/xgcc ] ; then \
      echo ../../gcc/xgcc -B../../gcc/; \
    else \
!     t='$(program_transform_name)'; echo gcc | sed -e '' $$t; \
    fi`
  
  NLMCONV_FOR_TARGET = ` \
    if [ -f ../../binutils/nlmconv ] ; then \
      echo ../../binutils/nlmconv; \
    else \
!     t='$(program_transform_name)'; echo nlmconv | sed -e '' $$t; \
    fi`
  
  # All the includes used for CFLAGS and for lint.
--- 55,68 ----
    if [ -f ../../gcc/xgcc ] ; then \
      echo ../../gcc/xgcc -B../../gcc/; \
    else \
!     t='$(program_transform_name)'; echo gcc | sed -e $$t; \
    fi`
  
  NLMCONV_FOR_TARGET = ` \
    if [ -f ../../binutils/nlmconv ] ; then \
      echo ../../binutils/nlmconv; \
    else \
!     t='$(program_transform_name)'; echo nlmconv | sed -e $$t; \
    fi`
  
  # All the includes used for CFLAGS and for lint.
Index: gdb/testsuite/Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/Makefile.in,v
retrieving revision 1.8
diff -c -r1.8 Makefile.in
*** gdb/testsuite/Makefile.in	23 Aug 2003 03:55:58 -0000	1.8
--- gdb/testsuite/Makefile.in	8 Oct 2003 19:22:48 -0000
***************
*** 53,59 ****
      if [ "$(host_canonical)" = "$(target_canonical)" ]; then \
        echo runtest; \
      else \
!       t='$(program_transform_name)'; echo runtest | sed -e '' $$t; \
      fi; \
    fi`
  
--- 53,59 ----
      if [ "$(host_canonical)" = "$(target_canonical)" ]; then \
        echo runtest; \
      else \
!       t='$(program_transform_name)'; echo runtest | sed -e $$t; \
      fi; \
    fi`
  

Re: RFC: Use program_transform_name correctly

[Andreas Schwab]

Daniel Jacobowitz <drow@mvista.com> writes:

> On Wed, Oct 08, 2003 at 02:17:26PM -0500, Jim Blandy wrote:
>> 
>> Daniel Jacobowitz <drow@mvista.com> writes:
>> > Conveniently this is no longer an issue in the src repository (or in
>> > gcc).  The toplevel configure script now guarantees that
>> > $(program_transform_name) will not be empty.  So $$t it is.
>> 
>> But doesn't that mean I can just say:
>> 
>>         sed '$(program_transform_name)'
>> 
>> since Make substitution doesn't respect single quotes in shell
>> commands?
>
> I'm not sure.  This is all complicated by the way that things get
> escaped if $(program_transform_name) contains a ' or a \.  And sed
> scripts often do contain backslashes:
>   --program-transform-name='s,(gdb),\1-6.0,'

Single quotes won't work in either case, whether you write

  t='$(program_transform_name)'; sed $$t

or

  sed '$(program_transform_name)'

but in the first case word splitting is performed on the sed argument.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Deutschherrnstr. 15-19, D-90429 Nürnberg
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: RFC: Use program_transform_name correctly

[Theodore A. Roth]

On Wed, 8 Oct 2003, Jim Blandy wrote:

>
> Could someone proofread this boring patch very carefully?

As long as gdb is using a autoconf-2.13 generated configure script, I
think that this will still have a problem when the user tries to pass
both --program-prefix=PREFIX and --program-suffix=SUFFIX to configure
at the same time. That was why I had added the double quotes around
$$t in the install-only rule.

Ted Roth

>
> gdb/Makefile.in:
> 2003-10-08  Jim Blandy  <jimb@redhat.com>
>
> 	* Makefile.in (CC_FOR_TARGET, CXX_FOR_TARGET, install-only): Use
> 	program_transform_name properly.
> 	* nlm/Makefile.in (CC_FOR_TARGET, NLMCONV_FOR_TARGET): Same.
>
> gdb/gdbserver/Makefile.in:
> 2003-10-08  Jim Blandy  <jimb@redhat.com>
>
> 	* Makefile.in (install-only, uninstall): Use
> 	program_transform_name properly.
>
> gdb/testsuite/Makefile.in:
> 2003-10-08  Jim Blandy  <jimb@redhat.com>
>
> 	* Makefile.in (RUNTEST_FOR_TARGET): Use program_transform_name
> 	properly.
>
> Index: gdb/Makefile.in
> ===================================================================
> RCS file: /cvs/src/src/gdb/Makefile.in,v
> retrieving revision 1.454
> diff -c -r1.454 Makefile.in
> *** gdb/Makefile.in	8 Oct 2003 02:41:49 -0000	1.454
> --- gdb/Makefile.in	8 Oct 2003 19:22:39 -0000
> ***************
> *** 465,471 ****
>       if [ "$(host_canonical)" = "$(target_canonical)" ] ; then \
>         echo $(CC); \
>       else \
> !       t='$(program_transform_name)'; echo gcc | sed -e '' $$t; \
>       fi; \
>     fi`
>
> --- 465,471 ----
>       if [ "$(host_canonical)" = "$(target_canonical)" ] ; then \
>         echo $(CC); \
>       else \
> !       t='$(program_transform_name)'; echo gcc | sed -e $$t; \
>       fi; \
>     fi`
>
> ***************
> *** 481,487 ****
>       if [ "$(host_canonical)" = "$(target_canonical)" ] ; then \
>         echo $(CXX); \
>       else \
> !       t='$(program_transform_name)'; echo gcc | sed -e '' $$t; \
>       fi; \
>     fi`
>
> --- 481,487 ----
>       if [ "$(host_canonical)" = "$(target_canonical)" ] ; then \
>         echo $(CXX); \
>       else \
> !       t='$(program_transform_name)'; echo gcc | sed -e $$t; \
>       fi; \
>     fi`
>
> ***************
> *** 963,969 ****
>   install: all install-only
>   install-only: $(CONFIG_INSTALL)
>   	transformed_name=`t='$(program_transform_name)'; \
> ! 			  echo gdb | sed -e "$$t"` ; \
>   		if test "x$$transformed_name" = x; then \
>   		  transformed_name=gdb ; \
>   		else \
> --- 963,969 ----
>   install: all install-only
>   install-only: $(CONFIG_INSTALL)
>   	transformed_name=`t='$(program_transform_name)'; \
> ! 			  echo gdb | sed -e $$t` ; \
>   		if test "x$$transformed_name" = x; then \
>   		  transformed_name=gdb ; \
>   		else \
> Index: gdb/gdbserver/Makefile.in
> ===================================================================
> RCS file: /cvs/src/src/gdb/gdbserver/Makefile.in,v
> retrieving revision 1.24
> diff -c -r1.24 Makefile.in
> *** gdb/gdbserver/Makefile.in	8 Aug 2003 17:30:36 -0000	1.24
> --- gdb/gdbserver/Makefile.in	8 Oct 2003 19:22:40 -0000
> ***************
> *** 149,155 ****
>   # install-only is intended to address that need.
>   install: all install-only
>   install-only:
> ! 	n=`echo gdbserver | sed '$(program_transform_name)'`; \
>   	if [ x$$n = x ]; then n=gdbserver; else true; fi; \
>   	$(SHELL) $(srcdir)/../../mkinstalldirs $(DESTDIR)$(bindir); \
>   	$(INSTALL_PROGRAM) gdbserver $(DESTDIR)$(bindir)/$$n; \
> --- 149,155 ----
>   # install-only is intended to address that need.
>   install: all install-only
>   install-only:
> ! 	n=`t='$(program_transform_name)'; echo gdbserver | sed $$t`; \
>   	if [ x$$n = x ]; then n=gdbserver; else true; fi; \
>   	$(SHELL) $(srcdir)/../../mkinstalldirs $(DESTDIR)$(bindir); \
>   	$(INSTALL_PROGRAM) gdbserver $(DESTDIR)$(bindir)/$$n; \
> ***************
> *** 157,163 ****
>   	$(INSTALL_DATA) $(srcdir)/gdbserver.1 $(DESTDIR)$(man1dir)/$$n.1
>
>   uninstall: force
> ! 	n=`echo gdbserver | sed '$(program_transform_name)'`; \
>   	if [ x$$n = x ]; then n=gdbserver; else true; fi; \
>   	rm -f $(bindir)/$$n $(DESTDIR)$(man1dir)/$$n.1
>
> --- 157,163 ----
>   	$(INSTALL_DATA) $(srcdir)/gdbserver.1 $(DESTDIR)$(man1dir)/$$n.1
>
>   uninstall: force
> ! 	n=`t='$(program_transform_name)'; echo gdbserver | sed $$t`; \
>   	if [ x$$n = x ]; then n=gdbserver; else true; fi; \
>   	rm -f $(bindir)/$$n $(DESTDIR)$(man1dir)/$$n.1
>
> Index: gdb/nlm/Makefile.in
> ===================================================================
> RCS file: /cvs/src/src/gdb/nlm/Makefile.in,v
> retrieving revision 1.2
> diff -c -r1.2 Makefile.in
> *** gdb/nlm/Makefile.in	6 Mar 2001 08:21:46 -0000	1.2
> --- gdb/nlm/Makefile.in	8 Oct 2003 19:22:40 -0000
> ***************
> *** 55,68 ****
>     if [ -f ../../gcc/xgcc ] ; then \
>       echo ../../gcc/xgcc -B../../gcc/; \
>     else \
> !     t='$(program_transform_name)'; echo gcc | sed -e '' $$t; \
>     fi`
>
>   NLMCONV_FOR_TARGET = ` \
>     if [ -f ../../binutils/nlmconv ] ; then \
>       echo ../../binutils/nlmconv; \
>     else \
> !     t='$(program_transform_name)'; echo nlmconv | sed -e '' $$t; \
>     fi`
>
>   # All the includes used for CFLAGS and for lint.
> --- 55,68 ----
>     if [ -f ../../gcc/xgcc ] ; then \
>       echo ../../gcc/xgcc -B../../gcc/; \
>     else \
> !     t='$(program_transform_name)'; echo gcc | sed -e $$t; \
>     fi`
>
>   NLMCONV_FOR_TARGET = ` \
>     if [ -f ../../binutils/nlmconv ] ; then \
>       echo ../../binutils/nlmconv; \
>     else \
> !     t='$(program_transform_name)'; echo nlmconv | sed -e $$t; \
>     fi`
>
>   # All the includes used for CFLAGS and for lint.
> Index: gdb/testsuite/Makefile.in
> ===================================================================
> RCS file: /cvs/src/src/gdb/testsuite/Makefile.in,v
> retrieving revision 1.8
> diff -c -r1.8 Makefile.in
> *** gdb/testsuite/Makefile.in	23 Aug 2003 03:55:58 -0000	1.8
> --- gdb/testsuite/Makefile.in	8 Oct 2003 19:22:48 -0000
> ***************
> *** 53,59 ****
>       if [ "$(host_canonical)" = "$(target_canonical)" ]; then \
>         echo runtest; \
>       else \
> !       t='$(program_transform_name)'; echo runtest | sed -e '' $$t; \
>       fi; \
>     fi`
>
> --- 53,59 ----
>       if [ "$(host_canonical)" = "$(target_canonical)" ]; then \
>         echo runtest; \
>       else \
> !       t='$(program_transform_name)'; echo runtest | sed -e $$t; \
>       fi; \
>     fi`
>
>

Re: RFC: Use program_transform_name correctly

[Felix Lee]

Jim Blandy <jimb@redhat.com>:
> But doesn't that mean I can just say:
>         sed '$(program_transform_name)'
> since Make substitution doesn't respect single quotes in shell
> commands?

try doing
    configure --program-transform-name="s/^/John's /"
I don't think that works right now.
--

Re: RFC: Use program_transform_name correctly

[Felix Lee]

"Theodore A. Roth" <troth@openavr.org>:
> As long as gdb is using a autoconf-2.13 generated configure script, I
> think that this will still have a problem when the user tries to pass
> both --program-prefix=PREFIX and --program-suffix=SUFFIX to configure
> at the same time. That was why I had added the double quotes around
> $$t in the install-only rule.

ah, right.  but doublequotes aren't necessarily any better...

hmm.  the autoconf 2.13 and 2.57 info files say to use
    transform=@program_transform_name@
    echo $$p|sed '$(transform)'

so use that, and fix things to make that work.  it doesn't really
matter what the quoting convention is as long as it's consistent.
--

Re: RFC: Use program_transform_name correctly

[Daniel Jacobowitz]

On Wed, Oct 08, 2003 at 02:30:48PM -0500, Jim Blandy wrote:
> 
> Could someone proofread this boring patch very carefully?

I don't know what the right thing to do is.  But have you tried a
couple of tests - configure, make, make install using, say:
  --program-suffix=a --program-prefix=b
  --program-transform-name='s/(g)/a\1b/'
  --program-prefix="A'B"

I don't even know if the top level autoconf script will let you do some
of these.  I suspect it won't, I found serious problems there before. 
Also, make sure /bin/sh is bash rather than ash; autoconf falls down
with backslashes in arguments otherwise.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer