diff mbox

tui: Simplify tui_alloc_content

Message ID 1442877416-16659-1-git-send-email-simon.marchi@ericsson.com
State New, archived
Headers

Commit Message

Simon Marchi Sept. 21, 2015, 11:16 p.m. UTC 
  I stumbled upon this while doing some cxx-conversion work.  Since the
x-family alloc functions throw on failure, it is useless to test their
result for failure.  The else branch of != NULL is basically dead code.

I changed the type of element_block_ptr to struct tui_win_element, which
seems obvious (this is actually what raised the flag, casting the result
of xmalloc to struct tui_win_element* wouldn't work).

gdb/ChangeLog:

	* tui/tui-data.c (tui_alloc_content): Don't check xmalloc
	result.  Change type of element_block_ptr.  Change allocation to
	use XNEWVEC.
---
 gdb/tui/tui-data.c | 41 ++++++++++++++---------------------------
 1 file changed, 14 insertions(+), 27 deletions(-)

Comments

Andrew Burgess Sept. 22, 2015, 7:50 a.m. UTC | #1 
* Simon Marchi <simon.marchi@ericsson.com> [2015-09-21 19:16:56 -0400]:

> 
> gdb/ChangeLog:
> 
> 	* tui/tui-data.c (tui_alloc_content): Don't check xmalloc
> 	result.  Change type of element_block_ptr.  Change allocation to
> 	use XNEWVEC.
> ---
>  gdb/tui/tui-data.c | 41 ++++++++++++++---------------------------
>  1 file changed, 14 insertions(+), 27 deletions(-)
> 
> diff --git a/gdb/tui/tui-data.c b/gdb/tui/tui-data.c
> index 2fcd547..ca7502d 100644
> --- a/gdb/tui/tui-data.c
> +++ b/gdb/tui/tui-data.c
> @@ -573,40 +573,27 @@ tui_win_content
>  tui_alloc_content (int num_elements, enum tui_win_type type)
>  {
>    tui_win_content content;
> -  char *element_block_ptr;
> +  struct tui_win_element *element_block_ptr;
>    int i;
>  
>    content = XNEWVEC (struct tui_win_element *, num_elements);
> -  if (content != NULL)
> +
> +  /*
> +   * All windows, except the data window, can allocate the
> +   * elements in a chunk.  The data window cannot because items
> +   * can be added/removed from the data display by the user at any
> +   * time.
> +   */
> +  if (type != DATA_WIN)
>      {
> -      /*
> -       * All windows, except the data window, can allocate the
> -       * elements in a chunk.  The data window cannot because items
> -       * can be added/removed from the data display by the user at any
> -       * time.
> -       */
> -      if (type != DATA_WIN)
> +      element_block_ptr = XNEWVEC (struct tui_win_element, num_elements);
> +      for (i = 0; i < num_elements; i++)
>  	{
> -	  element_block_ptr =
> -	    xmalloc (sizeof (struct tui_win_element) * num_elements);
> -	  if (element_block_ptr != NULL)
> -	    {
> -	      for (i = 0; i < num_elements; i++)
> -		{
> -		  content[i] = (struct tui_win_element *) element_block_ptr;
> -		  init_content_element (content[i], type);
> -		  element_block_ptr += sizeof (struct tui_win_element);
> -		}
> -	    }
> -	  else
> -	    {
> -	      xfree (content);
> -	      content = (tui_win_content) NULL;
> -	    }
> +	  content[i] = element_block_ptr;
> +	  init_content_element (content[i], type);
> +	  element_block_ptr++;
>  	}
>      }
> -
> -  return content;
>  }

Doesn't that leave the function without a return?

Thanks,
Andrew
Simon Marchi Sept. 22, 2015, 2:53 p.m. UTC | #2 
On 15-09-22 03:50 AM, Andrew Burgess wrote:
> * Simon Marchi <simon.marchi@ericsson.com> [2015-09-21 19:16:56 -0400]:
> 
>>
>> gdb/ChangeLog:
>>
>> 	* tui/tui-data.c (tui_alloc_content): Don't check xmalloc
>> 	result.  Change type of element_block_ptr.  Change allocation to
>> 	use XNEWVEC.
>> ---
>>  gdb/tui/tui-data.c | 41 ++++++++++++++---------------------------
>>  1 file changed, 14 insertions(+), 27 deletions(-)
>>
>> diff --git a/gdb/tui/tui-data.c b/gdb/tui/tui-data.c
>> index 2fcd547..ca7502d 100644
>> --- a/gdb/tui/tui-data.c
>> +++ b/gdb/tui/tui-data.c
>> @@ -573,40 +573,27 @@ tui_win_content
>>  tui_alloc_content (int num_elements, enum tui_win_type type)
>>  {
>>    tui_win_content content;
>> -  char *element_block_ptr;
>> +  struct tui_win_element *element_block_ptr;
>>    int i;
>>  
>>    content = XNEWVEC (struct tui_win_element *, num_elements);
>> -  if (content != NULL)
>> +
>> +  /*
>> +   * All windows, except the data window, can allocate the
>> +   * elements in a chunk.  The data window cannot because items
>> +   * can be added/removed from the data display by the user at any
>> +   * time.
>> +   */
>> +  if (type != DATA_WIN)
>>      {
>> -      /*
>> -       * All windows, except the data window, can allocate the
>> -       * elements in a chunk.  The data window cannot because items
>> -       * can be added/removed from the data display by the user at any
>> -       * time.
>> -       */
>> -      if (type != DATA_WIN)
>> +      element_block_ptr = XNEWVEC (struct tui_win_element, num_elements);
>> +      for (i = 0; i < num_elements; i++)
>>  	{
>> -	  element_block_ptr =
>> -	    xmalloc (sizeof (struct tui_win_element) * num_elements);
>> -	  if (element_block_ptr != NULL)
>> -	    {
>> -	      for (i = 0; i < num_elements; i++)
>> -		{
>> -		  content[i] = (struct tui_win_element *) element_block_ptr;
>> -		  init_content_element (content[i], type);
>> -		  element_block_ptr += sizeof (struct tui_win_element);
>> -		}
>> -	    }
>> -	  else
>> -	    {
>> -	      xfree (content);
>> -	      content = (tui_win_content) NULL;
>> -	    }
>> +	  content[i] = element_block_ptr;
>> +	  init_content_element (content[i], type);
>> +	  element_block_ptr++;
>>  	}
>>      }
>> -
>> -  return content;
>>  }
> 
> Doesn't that leave the function without a return?
> 
> Thanks,
> Andrew

Oops, of course the return needs to stay, otherwise it doesn't even
build.  I have it fixed locally but forgot to amend the commit.

Thanks!
Simon Marchi Oct. 26, 2015, 2:11 p.m. UTC | #3 
On 15-09-21 07:16 PM, Simon Marchi wrote:
> I stumbled upon this while doing some cxx-conversion work.  Since the
> x-family alloc functions throw on failure, it is useless to test their
> result for failure.  The else branch of != NULL is basically dead code.
> 
> I changed the type of element_block_ptr to struct tui_win_element, which
> seems obvious (this is actually what raised the flag, casting the result
> of xmalloc to struct tui_win_element* wouldn't work).
> 
> gdb/ChangeLog:
> 
> 	* tui/tui-data.c (tui_alloc_content): Don't check xmalloc
> 	result.  Change type of element_block_ptr.  Change allocation to
> 	use XNEWVEC.
> ---
>  gdb/tui/tui-data.c | 41 ++++++++++++++---------------------------
>  1 file changed, 14 insertions(+), 27 deletions(-)
> 
> diff --git a/gdb/tui/tui-data.c b/gdb/tui/tui-data.c
> index 2fcd547..ca7502d 100644
> --- a/gdb/tui/tui-data.c
> +++ b/gdb/tui/tui-data.c
> @@ -573,40 +573,27 @@ tui_win_content
>  tui_alloc_content (int num_elements, enum tui_win_type type)
>  {
>    tui_win_content content;
> -  char *element_block_ptr;
> +  struct tui_win_element *element_block_ptr;
>    int i;
>  
>    content = XNEWVEC (struct tui_win_element *, num_elements);
> -  if (content != NULL)
> +
> +  /*
> +   * All windows, except the data window, can allocate the
> +   * elements in a chunk.  The data window cannot because items
> +   * can be added/removed from the data display by the user at any
> +   * time.
> +   */
> +  if (type != DATA_WIN)
>      {
> -      /*
> -       * All windows, except the data window, can allocate the
> -       * elements in a chunk.  The data window cannot because items
> -       * can be added/removed from the data display by the user at any
> -       * time.
> -       */
> -      if (type != DATA_WIN)
> +      element_block_ptr = XNEWVEC (struct tui_win_element, num_elements);
> +      for (i = 0; i < num_elements; i++)
>  	{
> -	  element_block_ptr =
> -	    xmalloc (sizeof (struct tui_win_element) * num_elements);
> -	  if (element_block_ptr != NULL)
> -	    {
> -	      for (i = 0; i < num_elements; i++)
> -		{
> -		  content[i] = (struct tui_win_element *) element_block_ptr;
> -		  init_content_element (content[i], type);
> -		  element_block_ptr += sizeof (struct tui_win_element);
> -		}
> -	    }
> -	  else
> -	    {
> -	      xfree (content);
> -	      content = (tui_win_content) NULL;
> -	    }
> +	  content[i] = element_block_ptr;
> +	  init_content_element (content[i], type);
> +	  element_block_ptr++;
>  	}
>      }
> -
> -  return content;
>  }

Ping.
Pedro Alves Oct. 26, 2015, 4:33 p.m. UTC | #4 
On 10/26/2015 02:11 PM, Simon Marchi wrote:
> On 15-09-21 07:16 PM, Simon Marchi wrote:
>> I stumbled upon this while doing some cxx-conversion work.  Since the
>> x-family alloc functions throw on failure, it is useless to test their
>> result for failure.  The else branch of != NULL is basically dead code.
>>
>> I changed the type of element_block_ptr to struct tui_win_element, which
>> seems obvious (this is actually what raised the flag, casting the result
>> of xmalloc to struct tui_win_element* wouldn't work).
>>
>> gdb/ChangeLog:
>>
>> 	* tui/tui-data.c (tui_alloc_content): Don't check xmalloc
>> 	result.  Change type of element_block_ptr.  Change allocation to
>> 	use XNEWVEC.
>> ---
>>  gdb/tui/tui-data.c | 41 ++++++++++++++---------------------------
>>  1 file changed, 14 insertions(+), 27 deletions(-)
>>
>> diff --git a/gdb/tui/tui-data.c b/gdb/tui/tui-data.c
>> index 2fcd547..ca7502d 100644
>> --- a/gdb/tui/tui-data.c
>> +++ b/gdb/tui/tui-data.c
>> @@ -573,40 +573,27 @@ tui_win_content
>>  tui_alloc_content (int num_elements, enum tui_win_type type)
>>  {
>>    tui_win_content content;
>> -  char *element_block_ptr;
>> +  struct tui_win_element *element_block_ptr;
>>    int i;
>>  
>>    content = XNEWVEC (struct tui_win_element *, num_elements);
>> -  if (content != NULL)
>> +
>> +  /*
>> +   * All windows, except the data window, can allocate the
>> +   * elements in a chunk.  The data window cannot because items
>> +   * can be added/removed from the data display by the user at any
>> +   * time.
>> +   */
>> +  if (type != DATA_WIN)
>>      {
>> -      /*
>> -       * All windows, except the data window, can allocate the
>> -       * elements in a chunk.  The data window cannot because items
>> -       * can be added/removed from the data display by the user at any
>> -       * time.
>> -       */
>> -      if (type != DATA_WIN)
>> +      element_block_ptr = XNEWVEC (struct tui_win_element, num_elements);
>> +      for (i = 0; i < num_elements; i++)
>>  	{
>> -	  element_block_ptr =
>> -	    xmalloc (sizeof (struct tui_win_element) * num_elements);
>> -	  if (element_block_ptr != NULL)
>> -	    {
>> -	      for (i = 0; i < num_elements; i++)
>> -		{
>> -		  content[i] = (struct tui_win_element *) element_block_ptr;
>> -		  init_content_element (content[i], type);
>> -		  element_block_ptr += sizeof (struct tui_win_element);
>> -		}
>> -	    }
>> -	  else
>> -	    {
>> -	      xfree (content);
>> -	      content = (tui_win_content) NULL;
>> -	    }
>> +	  content[i] = element_block_ptr;
>> +	  init_content_element (content[i], type);
>> +	  element_block_ptr++;
>>  	}
>>      }
>> -
>> -  return content;
>>  }
> 
> Ping.
> 

OK with the return statement kept.

Thanks,
Pedro Alves
Simon Marchi Oct. 26, 2015, 4:59 p.m. UTC | #5 
On 15-10-26 12:33 PM, Pedro Alves wrote:
> OK with the return statement kept.
> 
> Thanks,
> Pedro Alves

Pushed with the return statement kept, thanks.
diff mbox

Patch

diff --git a/gdb/tui/tui-data.c b/gdb/tui/tui-data.c
index 2fcd547..ca7502d 100644
--- a/gdb/tui/tui-data.c
+++ b/gdb/tui/tui-data.c
@@ -573,40 +573,27 @@  tui_win_content
 tui_alloc_content (int num_elements, enum tui_win_type type)
 {
   tui_win_content content;
-  char *element_block_ptr;
+  struct tui_win_element *element_block_ptr;
   int i;
 
   content = XNEWVEC (struct tui_win_element *, num_elements);
-  if (content != NULL)
+
+  /*
+   * All windows, except the data window, can allocate the
+   * elements in a chunk.  The data window cannot because items
+   * can be added/removed from the data display by the user at any
+   * time.
+   */
+  if (type != DATA_WIN)
     {
-      /*
-       * All windows, except the data window, can allocate the
-       * elements in a chunk.  The data window cannot because items
-       * can be added/removed from the data display by the user at any
-       * time.
-       */
-      if (type != DATA_WIN)
+      element_block_ptr = XNEWVEC (struct tui_win_element, num_elements);
+      for (i = 0; i < num_elements; i++)
 	{
-	  element_block_ptr =
-	    xmalloc (sizeof (struct tui_win_element) * num_elements);
-	  if (element_block_ptr != NULL)
-	    {
-	      for (i = 0; i < num_elements; i++)
-		{
-		  content[i] = (struct tui_win_element *) element_block_ptr;
-		  init_content_element (content[i], type);
-		  element_block_ptr += sizeof (struct tui_win_element);
-		}
-	    }
-	  else
-	    {
-	      xfree (content);
-	      content = (tui_win_content) NULL;
-	    }
+	  content[i] = element_block_ptr;
+	  init_content_element (content[i], type);
+	  element_block_ptr++;
 	}
     }
-
-  return content;
 }