apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Stein <gst...@lyra.org>
Subject Re: cvs commit: apr/shmem/beos shmem.c
Date Thu, 07 Jun 2001 03:37:45 GMT
On Wed, Jun 06, 2001 at 09:52:45PM -0000, dreid@apache.org wrote:
>...
>   --- shmem.c	2001/04/30 20:55:17	1.1
>   +++ shmem.c	2001/06/06 21:52:42	1.2
>   @@ -57,6 +57,7 @@
>    #include "apr_errno.h"
>    #include "apr_lib.h"
>    #include "apr_strings.h"
>   +#include <stdio.h>

What is stdio for? I wouldn't think that to be necessary here.

>...
>   @@ -204,7 +207,7 @@
>    static void free_block(struct shmem_t *m, void *entity)
>    {
>        struct block_t *b;
>   -    if (b = find_block_by_addr(m->uselist, entity)){
>   +    if ((b = find_block_by_addr(m->uselist, entity))){

This would be clearer to write:

  if ((b = ...) != NULL)

Just throwing in parens is likely to hit the case one day, where somebody
"optimizes" the parens out of there and we get the compiler warning again.
Or even worse, if somebody thinks it should be "b == ...". The "!= NULL"
makes it quite explicit what is needed / being-done.

>...
>   @@ -278,7 +278,7 @@
>    void *apr_shm_malloc(struct shmem_t *m, apr_size_t reqsize)
>    {
>        struct block_t *b;
>   -    if (b = alloc_block(m, reqsize))
>   +    if ((b = alloc_block(m, reqsize)))
>            return b->addr;
>        return NULL;
>    }
>   @@ -286,7 +286,7 @@
>    void *apr_shm_calloc(struct shmem_t *m, apr_size_t reqsize)
>    {
>        struct block_t *b; 
>   -    if (b = alloc_block(m, reqsize)){  
>   +    if ((b = alloc_block(m, reqsize))){  

Same comment for these.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Mime
View raw message