[PATCH] Refine read_string

[PATCH] Refine read_string

[Yao Qi]

In read_string, we have this line

  chunksize = (len == -1 ? min (8, fetchlimit) : fetchlimit);

but chunksize is only used in the block that lne == -1, so IWBN to
move chunksize to the block in which it is used, and simplify the
condition setting chunksize.  This patch also moves 'found_nul' to
inner block.  This patch also splits a paragraph of comment into two,
and move them to different condition blocks (len > 0 and len == -1)
respectively.

Rebuild GDB on x86-linux.  Is it OK?

gdb:

2014-08-21  Yao Qi  <yao@codesourcery.com>

	* valprint.c (read_string): Move local variables 'found_nul',
	'chunksize' and 'limit' to inner scope.  Update comments.
---
 gdb/valprint.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/gdb/valprint.c b/gdb/valprint.c
index 8cb5c74..315e455 100644
--- a/gdb/valprint.c
+++ b/gdb/valprint.c
@@ -1794,36 +1794,23 @@ int
 read_string (CORE_ADDR addr, int len, int width, unsigned int fetchlimit,
 	     enum bfd_endian byte_order, gdb_byte **buffer, int *bytes_read)
 {
-  int found_nul;		/* Non-zero if we found the nul char.  */
   int errcode;			/* Errno returned from bad reads.  */
   unsigned int nfetch;		/* Chars to fetch / chars fetched.  */
-  unsigned int chunksize;	/* Size of each fetch, in chars.  */
   gdb_byte *bufptr;		/* Pointer to next available byte in
 				   buffer.  */
-  gdb_byte *limit;		/* First location past end of fetch buffer.  */
   struct cleanup *old_chain = NULL;	/* Top of the old cleanup chain.  */
 
-  /* Decide how large of chunks to try to read in one operation.  This
-     is also pretty simple.  If LEN >= zero, then we want fetchlimit chars,
-     so we might as well read them all in one operation.  If LEN is -1, we
-     are looking for a NUL terminator to end the fetching, so we might as
-     well read in blocks that are large enough to be efficient, but not so
-     large as to be slow if fetchlimit happens to be large.  So we choose the
-     minimum of 8 and fetchlimit.  We used to use 200 instead of 8 but
-     200 is way too big for remote debugging over a serial line.  */
-
-  chunksize = (len == -1 ? min (8, fetchlimit) : fetchlimit);
-
   /* Loop until we either have all the characters, or we encounter
      some error, such as bumping into the end of the address space.  */
 
-  found_nul = 0;
   *buffer = NULL;
 
   old_chain = make_cleanup (free_current_contents, buffer);
 
   if (len > 0)
     {
+      /* We want fetchlimit chars, so we might as well read them all in
+	 one operation.  */
       unsigned int fetchlen = min (len, fetchlimit);
 
       *buffer = (gdb_byte *) xmalloc (fetchlen * width);
@@ -1837,6 +1824,18 @@ read_string (CORE_ADDR addr, int len, int width, unsigned int fetchlimit,
   else if (len == -1)
     {
       unsigned long bufsize = 0;
+      unsigned int chunksize;	/* Size of each fetch, in chars.  */
+      int found_nul;		/* Non-zero if we found the nul char.  */
+      gdb_byte *limit;		/* First location past end of fetch buffer.  */
+
+      found_nul = 0;
+      /* We are looking for a NUL terminator to end the fetching, so we
+	 might as well read in blocks that are large enough to be efficient,
+	 but not so large as to be slow if fetchlimit happens to be large.
+	 So we choose the minimum of 8 and fetchlimit.  We used to use 200
+	 instead of 8 but 200 is way too big for remote debugging over a
+	  serial line.  */
+      chunksize = min (8, fetchlimit);
 
       do
 	{
-- 
1.9.3

Re: [PATCH] Refine read_string

[Joel Brobecker]

Hi Yao,

> In read_string, we have this line
> 
>   chunksize = (len == -1 ? min (8, fetchlimit) : fetchlimit);
> 
> but chunksize is only used in the block that lne == -1, so IWBN to
> move chunksize to the block in which it is used, and simplify the
> condition setting chunksize.  This patch also moves 'found_nul' to
> inner block.  This patch also splits a paragraph of comment into two,
> and move them to different condition blocks (len > 0 and len == -1)
> respectively.
> 
> Rebuild GDB on x86-linux.  Is it OK?
> 
> gdb:
> 
> 2014-08-21  Yao Qi  <yao@codesourcery.com>
> 
> 	* valprint.c (read_string): Move local variables 'found_nul',
> 	'chunksize' and 'limit' to inner scope.  Update comments.

Sorry for the delay in reviewing this.

I like this small improvement. Would you mind applying it?

-- 
Joel

Re: [PATCH] Refine read_string

[Yao Qi]

Joel Brobecker <brobecker@adacore.com> writes:

> I like this small improvement. Would you mind applying it?

Joel,
I've pushed it in.

-- 
Yao (齐尧)