[RFA] cli/cli-dump.c: Write dump always in binary mode on Cygwin

[RFA] cli/cli-dump.c: Write dump always in binary mode on Cygwin

[Corinna Vinschen]

Hi,

the following patch solves a problem in the dump code in cli/cli-dump.c.
On systems which support binary and textmode file IO, the dump code
doesn't explicitly write in binary mode, resulting in files written
in textmode if that's the current default on the system.  This leads
to a corrupted dump file since each LF is converted to CR/LF in the
file.  The patch uses the "b" flag in the fopen(3) call to write
explicitly binary.  Even if that flag is POSIX standard, I decided
to use it only on systems supporting binmode/textmode so that also
older systems have a chance.

Corinna

2002-08-05  Corinna Vinschen  <vinschen@redhat.com>

	* cli/cli-dump.c: Include fcntl.h to get open(2) flags.
	(add_dump_command): Utilize "b" fopen(3) flag on systems
	defining O_BINARY.

Index: gdb/cli/cli-dump.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-dump.c,v
retrieving revision 1.4
diff -u -p -r1.4 cli-dump.c
--- gdb/cli/cli-dump.c	24 May 2002 01:25:52 -0000	1.4
+++ gdb/cli/cli-dump.c	5 Aug 2002 10:15:17 -0000
@@ -30,6 +30,7 @@
 #include "cli/cli-dump.h"
 #include "gdb_assert.h"
 #include <ctype.h>
+#include <fcntl.h>
 #include "target.h"
 
 #define XMALLOC(TYPE) ((TYPE*) xmalloc (sizeof (TYPE)))
@@ -442,7 +443,11 @@ add_dump_command (char *name, void (*fun
   c->completer =  filename_completer;
   d = XMALLOC (struct dump_context);
   d->func = func;
+#ifdef O_BINARY
+  d->mode = "wb";
+#else
   d->mode = "w";
+#endif
   set_cmd_context (c, d);
   c->func = call_dump_func;
 
@@ -450,7 +455,11 @@ add_dump_command (char *name, void (*fun
   c->completer =  filename_completer;
   d = XMALLOC (struct dump_context);
   d->func = func;
+#ifdef O_BINARY
+  d->mode = "ab";
+#else
   d->mode = "a";
+#endif
   set_cmd_context (c, d);
   c->func = call_dump_func;
 


-- 
Corinna Vinschen
Cygwin Developer
Red Hat, Inc.
mailto:vinschen@redhat.com

Re: [RFA] cli/cli-dump.c: Write dump always in binary mode on Cygwin

[Andreas Schwab]

Corinna Vinschen <vinschen@redhat.com> writes:

|> Hi,
|> 
|> the following patch solves a problem in the dump code in cli/cli-dump.c.
|> On systems which support binary and textmode file IO, the dump code
|> doesn't explicitly write in binary mode, resulting in files written
|> in textmode if that's the current default on the system.  This leads
|> to a corrupted dump file since each LF is converted to CR/LF in the
|> file.  The patch uses the "b" flag in the fopen(3) call to write
|> explicitly binary.  Even if that flag is POSIX standard

It's not only POSIX C but already ISO C.

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: [RFA] cli/cli-dump.c: Write dump always in binary mode on Cygwin

[Michael Snyder]

Corinna Vinschen wrote:
> 
> Hi,
> 
> the following patch solves a problem in the dump code in cli/cli-dump.c.
> On systems which support binary and textmode file IO, the dump code
> doesn't explicitly write in binary mode, resulting in files written
> in textmode if that's the current default on the system.  This leads
> to a corrupted dump file since each LF is converted to CR/LF in the
> file.  The patch uses the "b" flag in the fopen(3) call to write
> explicitly binary.  Even if that flag is POSIX standard, I decided
> to use it only on systems supporting binmode/textmode so that also
> older systems have a chance.
> 
> Corinna

Hmm, file has no assigned maintainer...
Well, as a global-write maintainer, I wouldn't be opposed to 
the way it's written, but do we actually know of any environments
where the #if is strictly necessary?  


> 2002-08-05  Corinna Vinschen  <vinschen@redhat.com>
> 
>         * cli/cli-dump.c: Include fcntl.h to get open(2) flags.
>         (add_dump_command): Utilize "b" fopen(3) flag on systems
>         defining O_BINARY.
> 
> Index: gdb/cli/cli-dump.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/cli/cli-dump.c,v
> retrieving revision 1.4
> diff -u -p -r1.4 cli-dump.c
> --- gdb/cli/cli-dump.c  24 May 2002 01:25:52 -0000      1.4
> +++ gdb/cli/cli-dump.c  5 Aug 2002 10:15:17 -0000
> @@ -30,6 +30,7 @@
>  #include "cli/cli-dump.h"
>  #include "gdb_assert.h"
>  #include <ctype.h>
> +#include <fcntl.h>
>  #include "target.h"
> 
>  #define XMALLOC(TYPE) ((TYPE*) xmalloc (sizeof (TYPE)))
> @@ -442,7 +443,11 @@ add_dump_command (char *name, void (*fun
>    c->completer =  filename_completer;
>    d = XMALLOC (struct dump_context);
>    d->func = func;
> +#ifdef O_BINARY
> +  d->mode = "wb";
> +#else
>    d->mode = "w";
> +#endif
>    set_cmd_context (c, d);
>    c->func = call_dump_func;
> 
> @@ -450,7 +455,11 @@ add_dump_command (char *name, void (*fun
>    c->completer =  filename_completer;
>    d = XMALLOC (struct dump_context);
>    d->func = func;
> +#ifdef O_BINARY
> +  d->mode = "ab";
> +#else
>    d->mode = "a";
> +#endif
>    set_cmd_context (c, d);
>    c->func = call_dump_func;
> 
> 
> --
> Corinna Vinschen
> Cygwin Developer
> Red Hat, Inc.
> mailto:vinschen@redhat.com

Re: [RFA] cli/cli-dump.c: Write dump always in binary mode on Cygwin

[Eli Zaretskii]

On Mon, 5 Aug 2002, Michael Snyder wrote:

> do we actually know of any environments
> where the #if is strictly necessary?  

Since GDB requires ISO C, and "wb" is part of the ISO C spec, I think we 
can drop the #if.  (Dependance on O_BINARY being defined is not very 
clean anyway, IMHO, since O_BINARY does not belong to `fopen'.)

Re: [RFA] cli/cli-dump.c: Write dump always in binary mode on Cygwin

[Corinna Vinschen]

On Tue, Aug 06, 2002 at 07:42:47AM +0300, Eli Zaretskii wrote:
> 
> On Mon, 5 Aug 2002, Michael Snyder wrote:
> 
> > do we actually know of any environments
> > where the #if is strictly necessary?  
> 
> Since GDB requires ISO C, and "wb" is part of the ISO C spec, I think we 
> can drop the #if.  (Dependance on O_BINARY being defined is not very 
> clean anyway, IMHO, since O_BINARY does not belong to `fopen'.)

Well, that changes the patch to:

2002-08-06  Corinna Vinschen  <vinschen@redhat.com>

        * cli/cli-dump.c (add_dump_command): Explicitely use "b" flag to
	write dump file binary.

Index: cli/cli-dump.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-dump.c,v
retrieving revision 1.4
diff -u -p -r1.4 cli-dump.c
--- cli/cli-dump.c      24 May 2002 01:25:52 -0000      1.4
+++ cli/cli-dump.c      6 Aug 2002 08:08:27 -0000
@@ -442,7 +442,7 @@ add_dump_command (char *name, void (*fun
   c->completer =  filename_completer;
   d = XMALLOC (struct dump_context);
   d->func = func;
-  d->mode = "w";
+  d->mode = "wb";
   set_cmd_context (c, d);
   c->func = call_dump_func;
 
@@ -450,7 +450,7 @@ add_dump_command (char *name, void (*fun
   c->completer =  filename_completer;
   d = XMALLOC (struct dump_context);
   d->func = func;
-  d->mode = "a";
+  d->mode = "ab";
   set_cmd_context (c, d);
   c->func = call_dump_func;
 

Is that ok?

Corinna

-- 
Corinna Vinschen
Cygwin Developer
Red Hat, Inc.
mailto:vinschen@redhat.com

Re: [RFA] cli/cli-dump.c: Write dump always in binary mode on Cygwin

[Eli Zaretskii]

On Tue, 6 Aug 2002, Corinna Vinschen wrote:

> -  d->mode = "w";
> +  d->mode = "wb";
>    set_cmd_context (c, d);
>    c->func = call_dump_func;
>  
> @@ -450,7 +450,7 @@ add_dump_command (char *name, void (*fun
>    c->completer =  filename_completer;
>    d = XMALLOC (struct dump_context);
>    d->func = func;
> -  d->mode = "a";
> +  d->mode = "ab";
>    set_cmd_context (c, d);
>    c->func = call_dump_func;
>  
> 
> Is that ok?

Fine with me, thanks.

Re: [RFA] cli/cli-dump.c: Write dump always in binary mode on Cygwin

[Corinna Vinschen]

On Tue, Aug 06, 2002 at 07:10:24PM +0300, Eli Zaretskii wrote:
> 
> On Tue, 6 Aug 2002, Corinna Vinschen wrote:
> 
> > -  d->mode = "w";
> > +  d->mode = "wb";
> >    set_cmd_context (c, d);
> >    c->func = call_dump_func;
> >  
> > @@ -450,7 +450,7 @@ add_dump_command (char *name, void (*fun
> >    c->completer =  filename_completer;
> >    d = XMALLOC (struct dump_context);
> >    d->func = func;
> > -  d->mode = "a";
> > +  d->mode = "ab";
> >    set_cmd_context (c, d);
> >    c->func = call_dump_func;
> >  
> > 
> > Is that ok?
> 
> Fine with me, thanks.

Thanks, applied.

Corinna

-- 
Corinna Vinschen
Cygwin Developer
Red Hat, Inc.
mailto:vinschen@redhat.com

Re: [RFA] cli/cli-dump.c: Write dump always in binary mode on Cygwin

[Andrew Cagney]

> On Mon, 5 Aug 2002, Michael Snyder wrote:
> 
> 
>> do we actually know of any environments
>> where the #if is strictly necessary?  
> 
> 
> Since GDB requires ISO C, and "wb" is part of the ISO C spec, I think we 
> can drop the #if.  (Dependance on O_BINARY being defined is not very 
> clean anyway, IMHO, since O_BINARY does not belong to `fopen'.)

Just to be pedantic.  GDB requires an ISO C syntax compiler but not the 
ISO C support libraries.

However, until there is evidence to the contrary, I agree we should 
assume it is going to work.

Andrew

Re: [RFA] cli/cli-dump.c: Write dump always in binary mode on Cygwin

[Richard Earnshaw]

> 
> On Mon, 5 Aug 2002, Michael Snyder wrote:
> 
> > do we actually know of any environments
> > where the #if is strictly necessary?  
> 
> Since GDB requires ISO C, and "wb" is part of the ISO C spec, I think we 
> can drop the #if.  (Dependance on O_BINARY being defined is not very 
> clean anyway, IMHO, since O_BINARY does not belong to `fopen'.)
> 

I don't think we can assume an ISO conforming C library.

R.

Re: [RFA] cli/cli-dump.c: Write dump always in binary mode on Cygwin

[Andrew Cagney]

>> 
>> On Mon, 5 Aug 2002, Michael Snyder wrote:
>> 
> 
>> > do we actually know of any environments
>> > where the #if is strictly necessary?  
> 
>> 
>> Since GDB requires ISO C, and "wb" is part of the ISO C spec, I think we 
>> can drop the #if.  (Dependance on O_BINARY being defined is not very 
>> clean anyway, IMHO, since O_BINARY does not belong to `fopen'.)
>> 
> 
> 
> I don't think we can assume an ISO conforming C library.

True.  However, I think we should at least start out with the assumption 
that the above will work.  We should wait until someone presents us with 
hard evidence that there is a problem before hacking around it :-)

enjoy,
Andrew

Re: [RFA] cli/cli-dump.c: Write dump always in binary mode on Cygwin

[Eli Zaretskii]

On Wed, 7 Aug 2002, Richard Earnshaw wrote:

> > Since GDB requires ISO C, and "wb" is part of the ISO C spec, I think we 
> > can drop the #if.  (Dependance on O_BINARY being defined is not very 
> > clean anyway, IMHO, since O_BINARY does not belong to `fopen'.)
> > 
> 
> I don't think we can assume an ISO conforming C library.

All we need to assume is that the `b' part does not cause `fopen' to 
fail.

Re: [RFA] cli/cli-dump.c: Write dump always in binary mode on Cygwin

[Andrew Cagney]

BTW,

NickC's just pointed Corinna and I at include/fopen-{bin,same}.c.  I 
guess GDB could just use those when it wants a binary file open.

Andrew

[RFA]: Change all fopen modes to binary in cli-dump.c [was Re: [RFA] cli/cli-dump.c: Write dump always in binary mode on Cygwin]

[Corinna Vinschen]

On Thu, Aug 08, 2002 at 12:57:13PM -0400, Andrew Cagney wrote:
> BTW,
> 
> NickC's just pointed Corinna and I at include/fopen-{bin,same}.c.  I 
> guess GDB could just use those when it wants a binary file open.

In the light of this I had a closer look to cli/cli-dump.c and
found that there are a lot of constand "w"'s "a"'s and "r"'s
used.  So the previous patch was pretty incomplete.  The below
patch substitutes every constant fopen mode in cli/cli-dump.c
by the appropriate binary open mode define from include/fopen-bin.h.

Ok to check in?

Corinna

2002-08-09  Corinna Vinschen  <vinschen@redhat.com>

	* cli/cli-dump.c: Change fopen modes to use binary open modes
	as defined in include/fopen-bin.h throughout.

Index: cli/cli-dump.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-dump.c,v
retrieving revision 1.5
diff -u -p -r1.5 cli-dump.c
--- cli/cli-dump.c	7 Aug 2002 08:52:24 -0000	1.5
+++ cli/cli-dump.c	9 Aug 2002 09:12:48 -0000
@@ -360,61 +360,61 @@ dump_filetype (char *cmd, char *mode, ch
 static void
 dump_srec_memory (char *args, int from_tty)
 {
-  dump_memory_to_file (args, "w", "srec");
+  dump_memory_to_file (args, FOPEN_WB, "srec");
 }
 
 static void
 dump_srec_value (char *args, int from_tty)
 {
-  dump_value_to_file (args, "w", "srec");
+  dump_value_to_file (args, FOPEN_WB, "srec");
 }
 
 static void
 dump_ihex_memory (char *args, int from_tty)
 {
-  dump_memory_to_file (args, "w", "ihex");
+  dump_memory_to_file (args, FOPEN_WB, "ihex");
 }
 
 static void
 dump_ihex_value (char *args, int from_tty)
 {
-  dump_value_to_file (args, "w", "ihex");
+  dump_value_to_file (args, FOPEN_WB, "ihex");
 }
 
 static void
 dump_tekhex_memory (char *args, int from_tty)
 {
-  dump_memory_to_file (args, "w", "tekhex");
+  dump_memory_to_file (args, FOPEN_WB, "tekhex");
 }
 
 static void
 dump_tekhex_value (char *args, int from_tty)
 {
-  dump_value_to_file (args, "w", "tekhex");
+  dump_value_to_file (args, FOPEN_WB, "tekhex");
 }
 
 static void
 dump_binary_memory (char *args, int from_tty)
 {
-  dump_memory_to_file (args, "w", "binary");
+  dump_memory_to_file (args, FOPEN_WB, "binary");
 }
 
 static void
 dump_binary_value (char *args, int from_tty)
 {
-  dump_value_to_file (args, "w", "binary");
+  dump_value_to_file (args, FOPEN_WB, "binary");
 }
 
 static void
 append_binary_memory (char *args, int from_tty)
 {
-  dump_memory_to_file (args, "a", "binary");
+  dump_memory_to_file (args, FOPEN_AB, "binary");
 }
 
 static void
 append_binary_value (char *args, int from_tty)
 {
-  dump_value_to_file (args, "a", "binary");
+  dump_value_to_file (args, FOPEN_AB, "binary");
 }
 
 struct dump_context
@@ -442,7 +442,7 @@ add_dump_command (char *name, void (*fun
   c->completer =  filename_completer;
   d = XMALLOC (struct dump_context);
   d->func = func;
-  d->mode = "wb";
+  d->mode = FOPEN_WB;
   set_cmd_context (c, d);
   c->func = call_dump_func;
 
@@ -450,7 +450,7 @@ add_dump_command (char *name, void (*fun
   c->completer =  filename_completer;
   d = XMALLOC (struct dump_context);
   d->func = func;
-  d->mode = "ab";
+  d->mode = FOPEN_AB;
   set_cmd_context (c, d);
   c->func = call_dump_func;
 
@@ -547,7 +547,7 @@ restore_section_callback (bfd *ibfd, ase
 static void
 restore_binary_file (char *filename, struct callback_data *data)
 {
-  FILE *file = fopen_with_cleanup (filename, "r");
+  FILE *file = fopen_with_cleanup (filename, FOPEN_RB);
   int status;
   char *buf;
   long len;

-- 
Corinna Vinschen
Cygwin Developer
Red Hat, Inc.
mailto:vinschen@redhat.com

Re: [RFA]: Change all fopen modes to binary in cli-dump.c [was Re: [RFA] cli/cli-dump.c: Write dump always in binary mode on Cygwin]

[Andrew Cagney]

> On Thu, Aug 08, 2002 at 12:57:13PM -0400, Andrew Cagney wrote:
> 
>> BTW,
>> 
>> NickC's just pointed Corinna and I at include/fopen-{bin,same}.c.  I 
>> guess GDB could just use those when it wants a binary file open.
> 
> 
> In the light of this I had a closer look to cli/cli-dump.c and
> found that there are a lot of constand "w"'s "a"'s and "r"'s
> used.  So the previous patch was pretty incomplete.  The below
> patch substitutes every constant fopen mode in cli/cli-dump.c
> by the appropriate binary open mode define from include/fopen-bin.h.
> 
> Ok to check in?
> 
> Corinna
> 
> 2002-08-09  Corinna Vinschen  <vinschen@redhat.com>
> 
> 	* cli/cli-dump.c: Change fopen modes to use binary open modes
> 	as defined in include/fopen-bin.h throughout.
> 

Yes, thanks!  (I think I'll try to figure out a way of ARIing this so 
that we don't forget again :-)

Andrew

Re: [RFA]: Change all fopen modes to binary in cli-dump.c [was Re: [RFA] cli/cli-dump.c: Write dump always in binary mode on Cygwin]

[Corinna Vinschen]

On Fri, Aug 09, 2002 at 10:46:52AM -0400, Andrew Cagney wrote:
> Yes, thanks!  (I think I'll try to figure out a way of ARIing this so 
> that we don't forget again :-)

Thanks, applied.

Corinna

-- 
Corinna Vinschen
Cygwin Developer
Red Hat, Inc.
mailto:vinschen@redhat.com