Re: cvs commit: apr/shmem/beos shmem.c

[Greg Stein <gs...@lyra.org>]

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/