[patch binutils/11065] [DllTool] DLL name from DEF file is ignored when using --output-exp option

[patch binutils/11065] [DllTool] DLL name from DEF file is ignored when using --output-exp option

[Kai Tietz]

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

Hello,

this patch addresses the bug described at 
http://sourceware.org/bugzilla/show_bug.cgi?id=11065 report. The logic 
should be IMHO that if --dllname is specified, it shall be used in any 
case. If the exp_name is used for determine the DLL name, then just in 
case there is no DLL name specified in the .def file.

ChangeLog

2010-11-23  Kai Tietz

        PR binutils/11065
        * dlltool.c (dll_name_set_by_exp_name): New variable.
        (def_name): Allow setting of dll_name by .def file.
        (def_library): Likewise.
        (main): Set dll_name_set_by_exp_name, if dll_name is
        set indirect by exp_name.

Tested for x86_64-w64-mingw32, i686-pc-cygwin, and i686-w64-mingw32. Ok 
for apply?

Regards,
Kai

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


[-- Attachment #2: dlltool_11065.diff --]
[-- Type: application/octet-stream, Size: 1611 bytes --]

Index: src/binutils/dlltool.c
===================================================================
--- src.orig/binutils/dlltool.c	2010-11-02 11:06:15.000000000 +0100
+++ src/binutils/dlltool.c	2010-11-23 15:15:28.094842200 +0100
@@ -399,6 +399,7 @@ typedef struct identify_data_t
 static char *head_label;
 static char *imp_name_lab;
 static char *dll_name;
+static int dll_name_set_by_exp_name;
 static int add_indirect = 0;
 static int add_underscore = 0;
 static int add_stdcall_underscore = 0;
@@ -1089,6 +1090,15 @@ def_name (const char *name, int base)
   if (d_is_dll)
     non_fatal (_("Can't have LIBRARY and NAME"));
 
+  if (dll_name_set_by_exp_name)
+    {
+      if (dll_name)
+        {
+          free (dll_name);
+          dll_name = NULL;
+        }
+      dll_name_set_by_exp_name = 0;
+    }
   /* If --dllname not provided, use the one in the DEF file.
      FIXME: Is this appropriate for executables?  */
   if (!dll_name)
@@ -1105,6 +1115,16 @@ def_library (const char *name, int base)
   if (d_is_exe)
     non_fatal (_("Can't have LIBRARY and NAME"));
 
+  if (dll_name_set_by_exp_name)
+    {
+      if (dll_name)
+        {
+          free (dll_name);
+          dll_name = NULL;
+        }
+      dll_name_set_by_exp_name = 0;
+    }
+
   /* If --dllname not provided, use the one in the DEF file.  */
   if (!dll_name)
     set_dll_name_from_def (name, 1);
@@ -4177,6 +4197,7 @@ main (int ac, char **av)
       dll_name = xmalloc (len);
       strcpy (dll_name, exp_basename);
       strcat (dll_name, ".dll");
+      dll_name_set_by_exp_name = 1;
     }
 
   if (as_name == NULL)

Re: [patch binutils/11065] [DllTool] DLL name from DEF file is ignored when using --output-exp option

[Kai Tietz]

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

binutils-owner@sourceware.org wrote on 23.11.2010 15:19:40:

> Hello,
> 
> this patch addresses the bug described at 
> http://sourceware.org/bugzilla/show_bug.cgi?id=11065 report. The logic 
> should be IMHO that if --dllname is specified, it shall be used in any 
> case. If the exp_name is used for determine the DLL name, then just in 
> case there is no DLL name specified in the .def file.
> 
> ChangeLog
> 
> 2010-11-23  Kai Tietz
> 
>         PR binutils/11065
>         * dlltool.c (dll_name_set_by_exp_name): New variable.
>         (def_name): Allow setting of dll_name by .def file.
>         (def_library): Likewise.
>         (main): Set dll_name_set_by_exp_name, if dll_name is
>         set indirect by exp_name.
> 
> Tested for x86_64-w64-mingw32, i686-pc-cygwin, and i686-w64-mingw32. Ok 
> for apply?

Corrected in patch a potential wrong call of free to none-allocated 
memory. So dll_name can't be free'ed and is just set unconditional to NULL 
in case of a dll-name specified via .def file.

Kai

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


[-- Attachment #2: dlltool_11065.diff --]
[-- Type: application/octet-stream, Size: 1461 bytes --]

Index: src/binutils/dlltool.c
===================================================================
--- src.orig/binutils/dlltool.c	2010-11-02 11:06:15.000000000 +0100
+++ src/binutils/dlltool.c	2010-11-23 15:50:16.868148400 +0100
@@ -399,6 +399,7 @@ typedef struct identify_data_t
 static char *head_label;
 static char *imp_name_lab;
 static char *dll_name;
+static int dll_name_set_by_exp_name;
 static int add_indirect = 0;
 static int add_underscore = 0;
 static int add_stdcall_underscore = 0;
@@ -1089,6 +1090,11 @@ def_name (const char *name, int base)
   if (d_is_dll)
     non_fatal (_("Can't have LIBRARY and NAME"));
 
+  if (dll_name_set_by_exp_name)
+    {
+      dll_name = NULL;
+      dll_name_set_by_exp_name = 0;
+    }
   /* If --dllname not provided, use the one in the DEF file.
      FIXME: Is this appropriate for executables?  */
   if (!dll_name)
@@ -1105,6 +1111,12 @@ def_library (const char *name, int base)
   if (d_is_exe)
     non_fatal (_("Can't have LIBRARY and NAME"));
 
+  if (dll_name_set_by_exp_name)
+    {
+      dll_name = NULL;
+      dll_name_set_by_exp_name = 0;
+    }
+
   /* If --dllname not provided, use the one in the DEF file.  */
   if (!dll_name)
     set_dll_name_from_def (name, 1);
@@ -4177,6 +4189,7 @@ main (int ac, char **av)
       dll_name = xmalloc (len);
       strcpy (dll_name, exp_basename);
       strcat (dll_name, ".dll");
+      dll_name_set_by_exp_name = 1;
     }
 
   if (as_name == NULL)

Re: [patch binutils/11065] [DllTool] DLL name from DEF file is ignored when using --output-exp option

[Dave Korn]

On 23/11/2010 14:54, Kai Tietz wrote:

>> this patch addresses the bug described at 
>> http://sourceware.org/bugzilla/show_bug.cgi?id=11065 report. The logic 
>> should be IMHO that if --dllname is specified, it shall be used in any 
>> case. If the exp_name is used for determine the DLL name, then just in 
>> case there is no DLL name specified in the .def file.

  That seems correct to me.

>>         * dlltool.c (dll_name_set_by_exp_name): New variable.
>>         (def_name): Allow setting of dll_name by .def file.
>>         (def_library): Likewise.
>>         (main): Set dll_name_set_by_exp_name, if dll_name is
>>         set indirect by exp_name.

  I can see one problem with this: it is *optional* to provide a name in a
LIBRARY or NAME def file directive, so you may end up discarding one set from
a def file when you zero out dll_name and then not having one from the
directive to replace it with.  Please add guards against that case.  (I can't
tell you off the top of my head if the name argument will be passed to
def_name/def_library as a NULL or as an empty string by the parser in the case
where no name is supplied, I'm afraid; don't suppose it would hurt to just
check for either.)

    cheers,
      DaveK

Re: [patch binutils/11065] [DllTool] DLL name from DEF file is ignored when using --output-exp option

[Kai Tietz]

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

Dave Korn <dave.korn.cygwin@gmail.com> wrote on 23.11.2010 20:56:30:

> On 23/11/2010 14:54, Kai Tietz wrote:
> 
> >> this patch addresses the bug described at 
> >> http://sourceware.org/bugzilla/show_bug.cgi?id=11065 report. The 
logic 
> >> should be IMHO that if --dllname is specified, it shall be used in 
any 
> >> case. If the exp_name is used for determine the DLL name, then just 
in 
> >> case there is no DLL name specified in the .def file.
> 
>   That seems correct to me.
> 
> >>         * dlltool.c (dll_name_set_by_exp_name): New variable.
> >>         (def_name): Allow setting of dll_name by .def file.
> >>         (def_library): Likewise.
> >>         (main): Set dll_name_set_by_exp_name, if dll_name is
> >>         set indirect by exp_name.
> 
>   I can see one problem with this: it is *optional* to provide a name in 
a
> LIBRARY or NAME def file directive, so you may end up discarding one set 
from
> a def file when you zero out dll_name and then not having one from the
> directive to replace it with.  Please add guards against that case.  (I 
can't
> tell you off the top of my head if the name argument will be passed to
> def_name/def_library as a NULL or as an empty string by the parser in 
the case
> where no name is supplied, I'm afraid; don't suppose it would hurt to 
just
> check for either.)
> 
>     cheers,
>       DaveK
> 

Ok, done. See attached patch. Ok for apply?

Regards,
Kai

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

[-- Attachment #2: dlltool_11065.diff --]
[-- Type: application/octet-stream, Size: 1505 bytes --]

Index: src/binutils/dlltool.c
===================================================================
--- src.orig/binutils/dlltool.c	2010-11-02 11:06:15.000000000 +0100
+++ src/binutils/dlltool.c	2010-11-24 09:58:02.664925800 +0100
@@ -399,6 +399,7 @@ typedef struct identify_data_t
 static char *head_label;
 static char *imp_name_lab;
 static char *dll_name;
+static int dll_name_set_by_exp_name;
 static int add_indirect = 0;
 static int add_underscore = 0;
 static int add_stdcall_underscore = 0;
@@ -1089,6 +1090,11 @@ def_name (const char *name, int base)
   if (d_is_dll)
     non_fatal (_("Can't have LIBRARY and NAME"));
 
+  if (dll_name_set_by_exp_name && name && *name != 0)
+    {
+      dll_name = NULL;
+      dll_name_set_by_exp_name = 0;
+    }
   /* If --dllname not provided, use the one in the DEF file.
      FIXME: Is this appropriate for executables?  */
   if (!dll_name)
@@ -1105,6 +1111,12 @@ def_library (const char *name, int base)
   if (d_is_exe)
     non_fatal (_("Can't have LIBRARY and NAME"));
 
+  if (dll_name_set_by_exp_name && name && *name != 0)
+    {
+      dll_name = NULL;
+      dll_name_set_by_exp_name = 0;
+    }
+
   /* If --dllname not provided, use the one in the DEF file.  */
   if (!dll_name)
     set_dll_name_from_def (name, 1);
@@ -4177,6 +4189,7 @@ main (int ac, char **av)
       dll_name = xmalloc (len);
       strcpy (dll_name, exp_basename);
       strcat (dll_name, ".dll");
+      dll_name_set_by_exp_name = 1;
     }
 
   if (as_name == NULL)

Re: [patch binutils/11065] [DllTool] DLL name from DEF file is ignored when using --output-exp option

[Kai Tietz]

PING

2010/11/24 Kai Tietz <Kai.Tietz@onevision.com>:
> Dave Korn <dave.korn.cygwin@gmail.com> wrote on 23.11.2010 20:56:30:
>
>> On 23/11/2010 14:54, Kai Tietz wrote:
>>
>> >> this patch addresses the bug described at
>> >> http://sourceware.org/bugzilla/show_bug.cgi?id=11065 report. The
> logic
>> >> should be IMHO that if --dllname is specified, it shall be used in
> any
>> >> case. If the exp_name is used for determine the DLL name, then just
> in
>> >> case there is no DLL name specified in the .def file.
>>
>>   That seems correct to me.
>>
>> >>         * dlltool.c (dll_name_set_by_exp_name): New variable.
>> >>         (def_name): Allow setting of dll_name by .def file.
>> >>         (def_library): Likewise.
>> >>         (main): Set dll_name_set_by_exp_name, if dll_name is
>> >>         set indirect by exp_name.
>>
>>   I can see one problem with this: it is *optional* to provide a name in
> a
>> LIBRARY or NAME def file directive, so you may end up discarding one set
> from
>> a def file when you zero out dll_name and then not having one from the
>> directive to replace it with.  Please add guards against that case.  (I
> can't
>> tell you off the top of my head if the name argument will be passed to
>> def_name/def_library as a NULL or as an empty string by the parser in
> the case
>> where no name is supplied, I'm afraid; don't suppose it would hurt to
> just
>> check for either.)
>>
>>     cheers,
>>       DaveK
>>
>
> Ok, done. See attached patch. Ok for apply?
>
> Regards,
> Kai
>
> |  (\_/)  This is Bunny. Copy and paste Bunny
> | (='.'=) into your signature to help him gain
> | (")_(") world domination.
>



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

Re: [patch binutils/11065] [DllTool] DLL name from DEF file is ignored when using --output-exp option

[Dave Korn]

On 29/11/2010 18:09, Kai Tietz wrote:
> PING

>>>>>         * dlltool.c (dll_name_set_by_exp_name): New variable.
>>>>>         (def_name): Allow setting of dll_name by .def file.
>>>>>         (def_library): Likewise.
>>>>>         (main): Set dll_name_set_by_exp_name, if dll_name is
>>>>>         set indirect by exp_name.

>>>   I can see one problem with this: 

>>>
>> Ok, done. See attached patch. Ok for apply?

  Oops, sorry.  Yes, please commit.

    cheers,
      DaveK

Re: [patch binutils/11065] [DllTool] DLL name from DEF file is ignored when using --output-exp option

[Kai Tietz]

2010/12/1 Dave Korn <dave.korn.cygwin@gmail.com>:
> On 29/11/2010 18:09, Kai Tietz wrote:
>> PING
>
>>>>>>         * dlltool.c (dll_name_set_by_exp_name): New variable.
>>>>>>         (def_name): Allow setting of dll_name by .def file.
>>>>>>         (def_library): Likewise.
>>>>>>         (main): Set dll_name_set_by_exp_name, if dll_name is
>>>>>>         set indirect by exp_name.
>
>>>>   I can see one problem with this:
>
>>>>
>>> Ok, done. See attached patch. Ok for apply?
>
>  Oops, sorry.  Yes, please commit.
>
>    cheers,
>      DaveK
>
>

Applied to head.

Thanks,
Kai

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