[commit] Order parameters "rw", not "wr"

[commit] Order parameters "rw", not "wr"

[Andrew Cagney]

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

Hello,

I've just tripped over two new interfaces where I (for reasons I don't 
know or remember) ordered a parameter pair "w-r" (write, read) instead 
of "r-w" (read, write).  When trying to implement of these interfaces I 
consistently got the parameter order backwards (read, write)(1).

The attached patch re-orders the xfer_partial interface so its 
read/write buffers are ordered read/write (i.e., sanely).  I'm doing it 
now before too many things start to depend on the interface.

argh,
Andrew

(1) I also got parameters to pread/pwrite wrong so its a pretty bad day 
all round.

[-- Attachment #2: diffs --]
[-- Type: text/plain, Size: 6816 bytes --]

2003-11-10  Andrew Cagney  <cagney@redhat.com>

	* target.h (struct target_ops): Order xfer buffer parameters "read
	write" not "write read".
	* bfd-target.c (target_bfd_xfer_partial): Update.
	* remote.c (remote_xfer_partial): Update.
	* inftarg.c (child_xfer_partial): Update.
	* target.c (default_xfer_partial): Update.
	(target_read_partial, target_write_partial): Update.
	(debug_to_xfer_partial): Update.

Index: bfd-target.c
===================================================================
RCS file: /cvs/src/src/gdb/bfd-target.c,v
retrieving revision 1.1
diff -u -r1.1 bfd-target.c
--- bfd-target.c	31 Oct 2003 19:19:51 -0000	1.1
+++ bfd-target.c	10 Nov 2003 20:42:33 -0000
@@ -71,8 +71,8 @@
 LONGEST
 target_bfd_xfer_partial (struct target_ops *ops,
 			 enum target_object object,
-			 const char *annex, const void *writebuf, 
-			 void *readbuf, ULONGEST offset, LONGEST len)
+			 const char *annex, void *readbuf,
+			 const void *writebuf, ULONGEST offset, LONGEST len)
 {
   switch (object)
     {
Index: inftarg.c
===================================================================
RCS file: /cvs/src/src/gdb/inftarg.c,v
retrieving revision 1.19
diff -u -r1.19 inftarg.c
--- inftarg.c	7 Nov 2003 21:33:37 -0000	1.19
+++ inftarg.c	10 Nov 2003 20:42:33 -0000
@@ -556,10 +556,9 @@
    memory transfers, fall back to the old memory xfer functions.  */
 
 static LONGEST
-child_xfer_partial (struct target_ops *ops,
-		    enum target_object object,
-		    const char *annex, const void *writebuf,
-		    void *readbuf, ULONGEST offset, LONGEST len)
+child_xfer_partial (struct target_ops *ops, enum target_object object,
+		    const char *annex, void *readbuf,
+		    const void *writebuf, ULONGEST offset, LONGEST len)
 {
   switch (object)
     {
@@ -577,13 +576,13 @@
 #ifndef NATIVE_XFER_UNWIND_TABLE
 #define NATIVE_XFER_UNWIND_TABLE(OPS,OBJECT,ANNEX,WRITEBUF,READBUF,OFFSET,LEN) (-1)
 #endif
-      return NATIVE_XFER_UNWIND_TABLE (ops, object, annex, writebuf,
-				       readbuf, offset, len);
+      return NATIVE_XFER_UNWIND_TABLE (ops, object, annex, readbuf, writebuf,
+				       offset, len);
 #endif
 
 #if 0
     case TARGET_OBJECT_AUXV:
-      return native_xfer_auxv (PIDGET (inferior_ptid), writebuf, readbuf,
+      return native_xfer_auxv (PIDGET (inferior_ptid), readbuf, writebuf,
 			       offset, len);
 #endif
 
Index: remote.c
===================================================================
RCS file: /cvs/src/src/gdb/remote.c,v
retrieving revision 1.121
diff -u -r1.121 remote.c
--- remote.c	31 Oct 2003 15:25:34 -0000	1.121
+++ remote.c	10 Nov 2003 20:42:35 -0000
@@ -5103,7 +5103,7 @@
 
 static LONGEST
 remote_xfer_partial (struct target_ops *ops, enum target_object object,
-		     const char *annex, const void *writebuf, void *readbuf,
+		     const char *annex, void *readbuf, const void *writebuf,
 		     ULONGEST offset, LONGEST len)
 {
   struct remote_state *rs = get_remote_state ();
Index: target.c
===================================================================
RCS file: /cvs/src/src/gdb/target.c,v
retrieving revision 1.65
diff -u -r1.65 target.c
--- target.c	6 Nov 2003 19:56:26 -0000	1.65
+++ target.c	10 Nov 2003 20:42:35 -0000
@@ -75,9 +75,9 @@
 
 static LONGEST default_xfer_partial (struct target_ops *ops,
 				     enum target_object object,
-				     const char *annex, const void *writebuf,
-				     void *readbuf, ULONGEST offset,
-				     LONGEST len);
+				     const char *annex, void *readbuf,
+				     const void *writebuf,
+				     ULONGEST offset, LONGEST len);
 
 /* Transfer LEN bytes between target address MEMADDR and GDB address
    MYADDR.  Returns 0 for success, errno code for failure (which
@@ -1074,10 +1074,9 @@
 /* More generic transfers.  */
 
 static LONGEST
-default_xfer_partial (struct target_ops *ops,
-		      enum target_object object,
-		      const char *annex, const void *writebuf,
-		      void *readbuf, ULONGEST offset, LONGEST len)
+default_xfer_partial (struct target_ops *ops, enum target_object object,
+		      const char *annex, void *readbuf, 
+		      const void *writebuf, ULONGEST offset, LONGEST len)
 {
   if (object == TARGET_OBJECT_MEMORY
       && ops->to_xfer_memory != NULL)
@@ -1109,7 +1108,7 @@
     }
   else if (ops->beneath != NULL)
     return ops->beneath->to_xfer_partial (ops->beneath, object, annex,
-					  writebuf, readbuf, offset, len);
+					  readbuf, writebuf, offset, len);
   else
     return -1;
 }
@@ -1127,7 +1126,7 @@
 		     ULONGEST offset, LONGEST len)
 {
   gdb_assert (ops->to_xfer_partial != NULL);
-  return ops->to_xfer_partial (ops, object, annex, NULL, buf, offset, len);
+  return ops->to_xfer_partial (ops, object, annex, buf, NULL, offset, len);
 }
 
 LONGEST
@@ -1137,7 +1136,7 @@
 		      ULONGEST offset, LONGEST len)
 {
   gdb_assert (ops->to_xfer_partial != NULL);
-  return ops->to_xfer_partial (ops, object, annex, buf, NULL, offset, len);
+  return ops->to_xfer_partial (ops, object, annex, NULL, buf, offset, len);
 }
 
 /* Wrappers to perform the full transfer.  */
@@ -2289,20 +2288,19 @@
 }
 
 static LONGEST
-debug_to_xfer_partial (struct target_ops *ops,
-		       enum target_object object,
-		       const char *annex, const void *writebuf,
-		       void *readbuf, ULONGEST offset, LONGEST len)
+debug_to_xfer_partial (struct target_ops *ops, enum target_object object,
+		       const char *annex, void *readbuf, const void *writebuf,
+		       ULONGEST offset, LONGEST len)
 {
   LONGEST retval;
 
   retval = debug_target.to_xfer_partial (&debug_target, object, annex,
-					 writebuf, readbuf, offset, len);
+					 readbuf, writebuf, offset, len);
 
   fprintf_unfiltered (gdb_stdlog,
 		      "target_xfer_partial (%d, %s, 0x%lx,  0x%lx,  0x%s, %s) = %s\n",
 		      (int) object, (annex ? annex : "(null)"),
-		      (long) writebuf, (long) readbuf, paddr_nz (offset),
+		      (long) readbuf, (long) writebuf, paddr_nz (offset),
 		      paddr_d (len), paddr_d (retval));
 
   return retval;
Index: target.h
===================================================================
RCS file: /cvs/src/src/gdb/target.h,v
retrieving revision 1.52
diff -u -r1.52 target.h
--- target.h	6 Nov 2003 02:52:27 -0000	1.52
+++ target.h	10 Nov 2003 20:42:35 -0000
@@ -410,9 +410,9 @@
        and target_write_partial for details of each variant.  One, and
        only one, of readbuf or writebuf must be non-NULL.  */
     LONGEST (*to_xfer_partial) (struct target_ops *ops,
-				enum target_object object,
-				const char *annex, const void *writebuf,
-				void *readbuf, ULONGEST offset, LONGEST len);
+				enum target_object object, const char *annex,
+				void *readbuf, const void *writebuf, 
+				ULONGEST offset, LONGEST len);
 
     int to_magic;
     /* Need sub-structure for target machine related rather than comm related?

Re: [commit] Order parameters "rw", not "wr"

[Richard Henderson]

On Mon, Nov 10, 2003 at 04:20:13PM -0500, Andrew Cagney wrote:
> The attached patch re-orders the xfer_partial interface so its 
> read/write buffers are ordered read/write (i.e., sanely).

Eh??  The entire standard C library uses write/read.  I don't
see what's so unusual or non-canonical about it...


r~

Re: [commit] Order parameters "rw", not "wr"

[Andrew Cagney]

> On Mon, Nov 10, 2003 at 04:20:13PM -0500, Andrew Cagney wrote:
> 
>> The attached patch re-orders the xfer_partial interface so its 
>> read/write buffers are ordered read/write (i.e., sanely).
> 
> 
> Eh??  The entire standard C library uses write/read.  I don't
> see what's so unusual or non-canonical about it...

O_RDWR Open for reading and writing
S_IRWXU 00700 user (file owner) has read, write and execute permission
drwxr-xr-x    2 cagney   cagney       8192 Nov 11 13:52 bin

Andrew

Re: [commit] Order parameters "rw", not "wr"

[Daniel Jacobowitz]

On Thu, Nov 13, 2003 at 10:27:36AM -0500, Andrew Cagney wrote:
> >On Mon, Nov 10, 2003 at 04:20:13PM -0500, Andrew Cagney wrote:
> >
> >>The attached patch re-orders the xfer_partial interface so its 
> >>read/write buffers are ordered read/write (i.e., sanely).
> >
> >
> >Eh??  The entire standard C library uses write/read.  I don't
> >see what's so unusual or non-canonical about it...
> 
> O_RDWR Open for reading and writing
> S_IRWXU 00700 user (file owner) has read, write and execute permission
> drwxr-xr-x    2 cagney   cagney       8192 Nov 11 13:52 bin

Those aren't arguments, just a couple occurances of "read, write", so I
fail to see the connection.  Consider memcpy, strcat, fgets, sprintf. 

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [commit] Order parameters "rw", not "wr"

[Andrew Cagney]

>> O_RDWR Open for reading and writing
>> S_IRWXU 00700 user (file owner) has read, write and execute permission
>> drwxr-xr-x    2 cagney   cagney       8192 Nov 11 13:52 bin
> 
> 
> Those aren't arguments, just a couple occurances of "read, write", so I
> fail to see the connection.  Consider memcpy, strcat, fgets, sprintf. 

What you list here have little if any relevance to the interfaces in 
question.

Andrew

Re: [commit] Order parameters "rw", not "wr"

[Daniel Jacobowitz]

On Thu, Nov 13, 2003 at 11:35:56AM -0500, Andrew Cagney wrote:
> 
> >>O_RDWR Open for reading and writing
> >>S_IRWXU 00700 user (file owner) has read, write and execute permission
> >>drwxr-xr-x    2 cagney   cagney       8192 Nov 11 13:52 bin
> >
> >
> >Those aren't arguments, just a couple occurances of "read, write", so I
> >fail to see the connection.  Consider memcpy, strcat, fgets, sprintf. 
> 
> What you list here have little if any relevance to the interfaces in 
> question.

There are, as far as I know, no examples of functions in the standard
library which take an output buffer last.  As much of a C convention as
there is suggests they belong at the beginning.

If you're going to ignore that very weak precedent, that's your
perogative, go right ahead.  But please don't claim that "O_RDWR" is in
any way relevant to the argument ordering on target_xfer_partial
either.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [commit] Order parameters "rw", not "wr"

[Andrew Cagney]

> On Thu, Nov 13, 2003 at 11:35:56AM -0500, Andrew Cagney wrote:
> 
>> 
> 
>> >>O_RDWR Open for reading and writing
>> >>S_IRWXU 00700 user (file owner) has read, write and execute permission
>> >>drwxr-xr-x    2 cagney   cagney       8192 Nov 11 13:52 bin
> 
>> >
>> >
>> >Those aren't arguments, just a couple occurances of "read, write", so I
>> >fail to see the connection.  Consider memcpy, strcat, fgets, sprintf. 
> 
>> 
>> What you list here have little if any relevance to the interfaces in 
>> question.
> 
> 
> There are, as far as I know, no examples of functions in the standard
> library which take an output buffer last.  As much of a C convention as
> there is suggests they belong at the beginning.
> 
> If you're going to ignore that very weak precedent, that's your
> perogative, go right ahead.  But please don't claim that "O_RDWR" is in
> any way relevant to the argument ordering on target_xfer_partial
> either.

The xfer functions in question take _three_ [relevant] parameters:
	- an object
	- a read param
	- a write param
while the examples you cite take only _two_ [relevant] parameters:
	- an object
	- a read XOR write param
where, in the second case, the parameters are ordered randomly vis:

- write/read, object first (strcat, sprintf, write):
	- the write object
	- a read param
- read/write, object first (write):
	- the read object
	- a write param
- read/write, object last (fwrite):
	- a read param
	- the write object
- write/read, object last (fgets)
	- a write param
	- the read object

Consequently, as I've repeatedly stated, I see no relevance.

Andrew

Re: [commit] Order parameters "rw", not "wr"

[Andrew Cagney]

Oh, as always, 30 seconds after spending an hr on a reply, I remember:

      int select(int nfds, fd_set *readfds, fd_set *writefds, fd_set 
*exceptfds, struct timeval *timeout);

with more than two parameters and the ordering:

- object
- read param
- write param

Andrew

Re: [commit] Order parameters "rw", not "wr"

[Daniel Jacobowitz]

On Thu, Nov 13, 2003 at 01:01:23PM -0500, Andrew Cagney wrote:
> Oh, as always, 30 seconds after spending an hr on a reply, I remember:
> 
>      int select(int nfds, fd_set *readfds, fd_set *writefds, fd_set 
> *exceptfds, struct timeval *timeout);
> 
> with more than two parameters and the ordering:
> 
> - object
> - read param
> - write param

I think we're overloading the meaning of "read param", possibly because
I don't understand the arguments to the _partial functions.

Select is actually ordered:
 - input parameter
 - four in/out parameters

But let's drop it.  I don't care how you order target_read_partial; it
has so many arguments I'll have to go look it up every time anyway.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer