[patch ld/dlltool]: Allow empty LIBRARY statement in .def file

[patch ld/dlltool]: Allow empty LIBRARY statement in .def file

[Kai Tietz]

Hi,

this patch relax behavior of dlltool and ld about .def file parsing so
that the name of LIBRARY statement is optional.

ChangeLog

binutils/
2012-07-25  Kai Tietz

        * defparse.y (command): Call def_library only if name isn't
NULL and not empty.

ld/

2012-07-25  Kai Tietz

        * deffilep.y (command): Call def_image_name only if name isn't
NULL and not empty.

Regression tested for x86_64-w64-mingw32, i686-w64-mingw32, and
i686-pc-cygwin.  ok for apply?

Regards,
Kai

Index: defparse.y
===================================================================
RCS file: /cvs/src/src/binutils/defparse.y,v
retrieving revision 1.14
diff -u -r1.14 defparse.y
--- defparse.y  24 Feb 2012 14:20:16 -0000      1.14
+++ defparse.y  25 Jul 2012 20:56:31 -0000
@@ -52,7 +52,12 @@

 command:
                NAME opt_name opt_base { def_name ($2, $3); }
-       |       LIBRARY opt_name opt_base option_list { def_library ($2, $3); }
+       |       LIBRARY opt_name opt_base option_list
+               {
+                 /* Ignore LIBRARY without argument, or empty name.  */
+                 if ($2 && $2[0] != 0)
+                   def_library ($2, $3);
+               }
        |       EXPORTS explist
        |       DESCRIPTION ID { def_description ($2);}
        |       STACKSIZE NUMBER opt_number { def_stacksize ($2, $3);}


-- 
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination

Re: [patch ld/dlltool]: Allow empty LIBRARY statement in .def file

[Christopher Faylor]

On Wed, Jul 25, 2012 at 11:13:02PM +0200, Kai Tietz wrote:
>Hi,
>
>this patch relax behavior of dlltool and ld about .def file parsing so
>that the name of LIBRARY statement is optional.
>
>ChangeLog
>
>binutils/
>2012-07-25  Kai Tietz
>
>        * defparse.y (command): Call def_library only if name isn't
>NULL and not empty.
>
>ld/
>
>2012-07-25  Kai Tietz
>
>        * deffilep.y (command): Call def_image_name only if name isn't
>NULL and not empty.
>
>Regression tested for x86_64-w64-mingw32, i686-w64-mingw32, and
>i686-pc-cygwin.  ok for apply?
>
>Regards,
>Kai
>
>Index: defparse.y
>===================================================================
>RCS file: /cvs/src/src/binutils/defparse.y,v
>retrieving revision 1.14
>diff -u -r1.14 defparse.y
>--- defparse.y  24 Feb 2012 14:20:16 -0000      1.14
>+++ defparse.y  25 Jul 2012 20:56:31 -0000
>@@ -52,7 +52,12 @@
>
> command:
>                NAME opt_name opt_base { def_name ($2, $3); }
>-       |       LIBRARY opt_name opt_base option_list { def_library ($2, $3); }
>+       |       LIBRARY opt_name opt_base option_list
>+               {
>+                 /* Ignore LIBRARY without argument, or empty name.  */
>+                 if ($2 && $2[0] != 0)
>+                   def_library ($2, $3);
>+               }

It seems like def_library() is also making decisions based on its first
parameter.  This test will short-circuit those tests.  It seems like
either this should all be handled in def_library or the tests in
def_library() should be deleted.

cgf

Re: [patch ld/dlltool]: Allow empty LIBRARY statement in .def file

[Kai Tietz]

2012/7/26 Christopher Faylor <cgf-use-the-mailinglist-please@sourceware.org>:
> On Wed, Jul 25, 2012 at 11:13:02PM +0200, Kai Tietz wrote:
>>Hi,
>>
>>this patch relax behavior of dlltool and ld about .def file parsing so
>>that the name of LIBRARY statement is optional.
>>
>>ChangeLog
>>
>>binutils/
>>2012-07-25  Kai Tietz
>>
>>        * defparse.y (command): Call def_library only if name isn't
>>NULL and not empty.
>>
>>ld/
>>
>>2012-07-25  Kai Tietz
>>
>>        * deffilep.y (command): Call def_image_name only if name isn't
>>NULL and not empty.
>>
>>Regression tested for x86_64-w64-mingw32, i686-w64-mingw32, and
>>i686-pc-cygwin.  ok for apply?
>>
>>Regards,
>>Kai
>>
>>Index: defparse.y
>>===================================================================
>>RCS file: /cvs/src/src/binutils/defparse.y,v
>>retrieving revision 1.14
>>diff -u -r1.14 defparse.y
>>--- defparse.y  24 Feb 2012 14:20:16 -0000      1.14
>>+++ defparse.y  25 Jul 2012 20:56:31 -0000
>>@@ -52,7 +52,12 @@
>>
>> command:
>>                NAME opt_name opt_base { def_name ($2, $3); }
>>-       |       LIBRARY opt_name opt_base option_list { def_library ($2, $3); }
>>+       |       LIBRARY opt_name opt_base option_list
>>+               {
>>+                 /* Ignore LIBRARY without argument, or empty name.  */
>>+                 if ($2 && $2[0] != 0)
>>+                   def_library ($2, $3);
>>+               }
>
> It seems like def_library() is also making decisions based on its first
> parameter.  This test will short-circuit those tests.  It seems like
> either this should all be handled in def_library or the tests in
> def_library() should be deleted.

Yes, you are right. I think best way here is to remove the
useless-code paths in def_library caused by this patch. In fact logic
in def_library depends on valid name-argument, so the short-circuit we
can do also in this function.

I'll prepare a patch for this.

Regards,
Kai

Re: [patch ld/dlltool]: Allow empty LIBRARY statement in .def file

[nick clifton]

Hi Kai,

> binutils/
> 2012-07-25  Kai Tietz
>
>          * defparse.y (command): Call def_library only if name isn't
> NULL and not empty.
>
> ld/
>
> 2012-07-25  Kai Tietz
>
>          * deffilep.y (command): Call def_image_name only if name isn't
> NULL and not empty.
>
> Regression tested for x86_64-w64-mingw32, i686-w64-mingw32, and
> i686-pc-cygwin.  ok for apply?

Probably. :-)  You only included that patch to defparse.y in your 
posting, and missed out the patch to deffilep.y.  I do not see any 
problems with the patch so far, but please could you post the full thing 
so that I can check it over.

Cheers
   Nick

Re: [patch ld/dlltool]: Allow empty LIBRARY statement in .def file

[Kai Tietz]

2012/7/26 nick clifton <nickc@redhat.com>:
> Hi Kai,
>
>
>> binutils/
>> 2012-07-25  Kai Tietz
>>
>>          * defparse.y (command): Call def_library only if name isn't
>> NULL and not empty.
>>
>> ld/
>>
>> 2012-07-25  Kai Tietz
>>
>>          * deffilep.y (command): Call def_image_name only if name isn't
>> NULL and not empty.
>>
>> Regression tested for x86_64-w64-mingw32, i686-w64-mingw32, and
>> i686-pc-cygwin.  ok for apply?
>
>
> Probably. :-)  You only included that patch to defparse.y in your posting,
> and missed out the patch to deffilep.y.  I do not see any problems with the
> patch so far, but please could you post the full thing so that I can check
> it over.
>
> Cheers
>   Nick

Ups, have I missed the ld part.  Here is the missing part of the patch.

Index: deffilep.y
===================================================================
RCS file: /cvs/src/src/ld/deffilep.y,v
retrieving revision 1.40
diff -u -r1.40 deffilep.y
--- deffilep.y  24 Feb 2012 14:20:19 -0000      1.40
+++ deffilep.y  25 Jul 2012 20:58:21 -0000
@@ -140,7 +140,11 @@

 command:
                NAME opt_name opt_base { def_image_name ($2, $3, 0); }
-       |       LIBRARY opt_name opt_base { def_image_name ($2, $3, 1); }
+       |       LIBRARY opt_name opt_base
+               {
+                 if ($2 && $2[0] != 0)
+                   def_image_name ($2, $3, 1);
+               }
        |       DESCRIPTION ID { def_description ($2);}
        |       STACKSIZE_K NUMBER opt_number { def_stacksize ($2, $3);}
        |       HEAPSIZE NUMBER opt_number { def_heapsize ($2, $3);}

Re: [patch ld/dlltool]: Allow empty LIBRARY statement in .def file

[nick clifton]

Hi Kai,

   Thanks - the full patch is approved.

Cheers
   Nick

Re: [patch ld/dlltool]: Allow empty LIBRARY statement in .def file

[Christopher Faylor]

On Thu, Jul 26, 2012 at 02:07:19PM +0100, nick clifton wrote:
>Hi Kai,
>
>   Thanks - the full patch is approved.

What about my observations?

http://sourceware.org/ml/binutils/2012-07/msg00200.html

Shouldn't those be addressed before anything is checked in?

cgf

Re: [patch ld/dlltool]: Allow empty LIBRARY statement in .def file

[nick clifton]

Hi Chris,

> What about my observations?
>
> http://sourceware.org/ml/binutils/2012-07/msg00200.html
>
> Shouldn't those be addressed before anything is checked in?

Yes they should.

Sorry about short-circuiting your observations.  I posted my approval 
before I saw your email. :-(

Cheers
   Nick