Re: svn commit: r314322 - head/lib/librss

2017-02-26 Thread Cy Schubert
In message 

Re: svn commit: r314322 - head/lib/librss

2017-02-26 Thread Warner Losh
> Then why even test for RC being NULL?

So rc->rss_bucket_map doesn't dereference a null pointer and core dump maybe?

Warner
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r314322 - head/lib/librss

2017-02-26 Thread Pedro Giffuni



On 2/26/2017 9:37 PM, Cy Schubert wrote:

In message <404d743b-735b-0605-5ab5-ccb95ce24...@freebsd.org>, Pedro
Giffuni wr
ites:

This is a multi-part message in MIME format.
--C3FC59CAC7D072A09BF70AAF
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit

Hello;

On 2/26/2017 8:51 PM, Cy Schubert wrote:

In message <201702270010.v1r0a1wm074...@repo.freebsd.org>, "Pedro F.
Giffuni" w
rites:

Author: pfg
Date: Mon Feb 27 00:10:00 2017
New Revision: 314322
URL: https://svnweb.freebsd.org/changeset/base/314322

Log:
librss: simplify some NULL checks.

MFC after:	1 week


Modified:
head/lib/librss/librss.c

Modified: head/lib/librss/librss.c
==

===

=
--- head/lib/librss/librss.cSun Feb 26 22:17:06 2017(r31432

1)

+++ head/lib/librss/librss.cMon Feb 27 00:10:00 2017(r31432

2)

@@ -244,10 +244,10 @@ rss_config_get(void)
return (rc);
   
   error:

-   if ((rc != NULL) && rc->rss_bucket_map)
+   if (rc != NULL) {
free(rc->rss_bucket_map);

What happens if rc is not NULL and rc->rss_bucket_map is NULL?

According the free(3): " If /ptr/ is *NULL*, no action occurs."


Good point.

Then why even test for RC being NULL?


If rc is NULL, there is a chance that rc->rss_bucket_map is random 
garbage and you might be "double freeing" something.



The only reason I can think of doing any test for NULL before free(3) is
that if the likelihood of *ptr being NULL is greater than the likelihood of
*ptr not being NULL then you save running the extra instructions to make
that determination in free(), e.g. pushes, call, return, pops.



The check for rc NULL is necessary, the only optimization is that having 
done the check we already know if we need to free(rc), so while we could 
leave the second free() outside the branch it is better to leave it 
within the "if" .


Pedro.

___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r314322 - head/lib/librss

2017-02-26 Thread Cy Schubert
In message <404d743b-735b-0605-5ab5-ccb95ce24...@freebsd.org>, Pedro 
Giffuni wr
ites:
> This is a multi-part message in MIME format.
> --C3FC59CAC7D072A09BF70AAF
> Content-Type: text/plain; charset=windows-1252; format=flowed
> Content-Transfer-Encoding: 7bit
> 
> Hello;
> 
> On 2/26/2017 8:51 PM, Cy Schubert wrote:
> > In message <201702270010.v1r0a1wm074...@repo.freebsd.org>, "Pedro F.
> > Giffuni" w
> > rites:
> >> Author: pfg
> >> Date: Mon Feb 27 00:10:00 2017
> >> New Revision: 314322
> >> URL: https://svnweb.freebsd.org/changeset/base/314322
> >>
> >> Log:
> >>librss: simplify some NULL checks.
> >>
> >>MFC after:  1 week
> >>
> >> Modified:
> >>head/lib/librss/librss.c
> >>
> >> Modified: head/lib/librss/librss.c
> >> ==
> ===
> >> =
> >> --- head/lib/librss/librss.c   Sun Feb 26 22:17:06 2017(r31432
> 1)
> >> +++ head/lib/librss/librss.c   Mon Feb 27 00:10:00 2017(r31432
> 2)
> >> @@ -244,10 +244,10 @@ rss_config_get(void)
> >>return (rc);
> >>   
> >>   error:
> >> -  if ((rc != NULL) && rc->rss_bucket_map)
> >> +  if (rc != NULL) {
> >>free(rc->rss_bucket_map);
> > What happens if rc is not NULL and rc->rss_bucket_map is NULL?
> 
> According the free(3): " If /ptr/ is *NULL*, no action occurs."
> 

Good point.

Then why even test for RC being NULL?

The only reason I can think of doing any test for NULL before free(3) is 
that if the likelihood of *ptr being NULL is greater than the likelihood of 
*ptr not being NULL then you save running the extra instructions to make 
that determination in free(), e.g. pushes, call, return, pops.


-- 
Cheers,
Cy Schubert 
FreeBSD UNIX:     Web:  http://www.FreeBSD.org

The need of the many outweighs the greed of the few.


___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r314322 - head/lib/librss

2017-02-26 Thread Pedro Giffuni

Hello;

On 2/26/2017 8:51 PM, Cy Schubert wrote:

In message <201702270010.v1r0a1wm074...@repo.freebsd.org>, "Pedro F.
Giffuni" w
rites:

Author: pfg
Date: Mon Feb 27 00:10:00 2017
New Revision: 314322
URL: https://svnweb.freebsd.org/changeset/base/314322

Log:
   librss: simplify some NULL checks.
   
   MFC after:	1 week


Modified:
   head/lib/librss/librss.c

Modified: head/lib/librss/librss.c
=
=
--- head/lib/librss/librss.cSun Feb 26 22:17:06 2017(r314321)
+++ head/lib/librss/librss.cMon Feb 27 00:10:00 2017(r314322)
@@ -244,10 +244,10 @@ rss_config_get(void)
return (rc);
  
  error:

-   if ((rc != NULL) && rc->rss_bucket_map)
+   if (rc != NULL) {
free(rc->rss_bucket_map);

What happens if rc is not NULL and rc->rss_bucket_map is NULL?


According the free(3): " If /ptr/ is *NULL*, no action occurs."

Pedro.

___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r314322 - head/lib/librss

2017-02-26 Thread Cy Schubert
In message <201702270010.v1r0a1wm074...@repo.freebsd.org>, "Pedro F. 
Giffuni" w
rites:
> Author: pfg
> Date: Mon Feb 27 00:10:00 2017
> New Revision: 314322
> URL: https://svnweb.freebsd.org/changeset/base/314322
> 
> Log:
>   librss: simplify some NULL checks.
>   
>   MFC after:  1 week
> 
> Modified:
>   head/lib/librss/librss.c
> 
> Modified: head/lib/librss/librss.c
> =
> =
> --- head/lib/librss/librss.c  Sun Feb 26 22:17:06 2017(r314321)
> +++ head/lib/librss/librss.c  Mon Feb 27 00:10:00 2017(r314322)
> @@ -244,10 +244,10 @@ rss_config_get(void)
>   return (rc);
>  
>  error:
> - if ((rc != NULL) && rc->rss_bucket_map)
> + if (rc != NULL) {
>   free(rc->rss_bucket_map);

What happens if rc is not NULL and rc->rss_bucket_map is NULL?

> - if (rc != NULL)
>   free(rc);
> + }
>   return (NULL);
>  }
>  
> 
> 


-- 
Cheers,
Cy Schubert 
FreeBSD UNIX:     Web:  http://www.FreeBSD.org

The need of the many outweighs the greed of the few.



___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


svn commit: r314322 - head/lib/librss

2017-02-26 Thread Pedro F. Giffuni
Author: pfg
Date: Mon Feb 27 00:10:00 2017
New Revision: 314322
URL: https://svnweb.freebsd.org/changeset/base/314322

Log:
  librss: simplify some NULL checks.
  
  MFC after:1 week

Modified:
  head/lib/librss/librss.c

Modified: head/lib/librss/librss.c
==
--- head/lib/librss/librss.cSun Feb 26 22:17:06 2017(r314321)
+++ head/lib/librss/librss.cMon Feb 27 00:10:00 2017(r314322)
@@ -244,10 +244,10 @@ rss_config_get(void)
return (rc);
 
 error:
-   if ((rc != NULL) && rc->rss_bucket_map)
+   if (rc != NULL) {
free(rc->rss_bucket_map);
-   if (rc != NULL)
free(rc);
+   }
return (NULL);
 }
 
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"