You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Aaron Bannert <aa...@clove.org> on 2001/09/19 09:34:20 UTC

Re: cvs commit: httpd-2.0/server/mpm/worker worker.c

On Wed, Sep 19, 2001 at 06:34:11AM -0000, jwoolley@apache.org wrote:
> jwoolley    01/09/18 23:34:11
> 
>   Modified:    server/mpm/worker worker.c
>   Log:
>   I was kinda hoping those (void)some_function() and (request_rec *)NULL
>   casts would go away before this committed, but alas I didn't say anything.
>   :-)  This gets rid of them and a few others just like them that I also
>   found in worker.c.

I don't know what the groupthink is on this, or if there is a precedent
set, but I think the (void)s make it obvious to the reader that we
are deliberately throwing away the return value.

As for the (request_rec *)NULL thing, either way is fine for me.

-aaron

>   
>   Revision  Changes    Path
>   1.25      +5 -8      httpd-2.0/server/mpm/worker/worker.c
>   
>   Index: worker.c
>   ===================================================================
>   RCS file: /home/cvs/httpd-2.0/server/mpm/worker/worker.c,v
>   retrieving revision 1.24
>   retrieving revision 1.25
>   diff -u -d -u -r1.24 -r1.25
>   --- worker.c	2001/09/19 05:58:09	1.24
>   +++ worker.c	2001/09/19 06:34:11	1.25
>   @@ -483,7 +483,7 @@
>        long conn_id = AP_ID_FROM_CHILD_THREAD(my_child_num, my_thread_num);
>        int csd;
>    
>   -    (void) apr_os_sock_get(&csd, sock);
>   +    apr_os_sock_get(&csd, sock);
>    
>        if (csd >= FD_SETSIZE) {
>            ap_log_error(APLOG_MARK, APLOG_NOERRNO|APLOG_WARNING, 0, NULL,
>   @@ -697,11 +697,9 @@
>    
>        free(ti);
>    
>   -    (void) ap_update_child_status(process_slot, thread_slot,
>   -                                  SERVER_STARTING, (request_rec *)NULL);
>   +    ap_update_child_status(process_slot, thread_slot, SERVER_STARTING, NULL);
>        while (!workers_may_exit) {
>   -        (void) ap_update_child_status(process_slot, thread_slot,
>   -                                      SERVER_READY, (request_rec *)NULL);
>   +        ap_update_child_status(process_slot, thread_slot, SERVER_READY, NULL);
>            rv = ap_queue_pop(worker_queue, &csd, &ptrans);
>            /* We get FD_QUEUE_EINTR whenever ap_queue_pop() has been interrupted
>             * from an explicit call to ap_queue_interrupt_all(). This allows
>   @@ -778,8 +776,7 @@
>    	    my_info->sd = 0;
>    	
>      	    /* We are creating threads right now */
>   -	    (void) ap_update_child_status(my_child_num, i, SERVER_STARTING, 
>   -	  			          (request_rec *) NULL);
>   +	    ap_update_child_status(my_child_num, i, SERVER_STARTING, NULL);
>                /* We let each thread update its own scoreboard entry.  This is
>                 * done because it lets us deal with tid better.
>    	     */
>   @@ -947,7 +944,7 @@
>            /* fork didn't succeed. Fix the scoreboard or else
>             * it will say SERVER_STARTING forever and ever
>             */
>   -        (void) ap_update_child_status(slot, 0, SERVER_DEAD, (request_rec *) NULL);
>   +        ap_update_child_status(slot, 0, SERVER_DEAD, NULL);
>    
>    	/* In case system resources are maxxed out, we don't want
>    	   Apache running away with the CPU trying to fork over and
>   
>   
>   

Re: cvs commit: httpd-2.0/server/mpm/worker worker.c

Posted by Bill Stoddard <bi...@wstoddard.com>.
> Bill Stoddard wrote:
> >
> > It's obvious we are not using the return either way.
>
> But without the cast, someone not conversant with the details
> won't know it normally returns a value; they might think it's
> a void function.
>
> > Less is better so dump the cast :-)
>
> I think the cast improves understanding, and it doesn't
> make any difference at the object-code level, so I say 'keep
> it.' :-)

I can go either way. My last bits on the subject....

Should someone be interested in using the return code, they would look at the function and
discover that it -does- produce a return code and from that point on, they will know this
bit of knowledge. For everyone else, not having to look at (void) all over the place
represents a slight reduction in code complexity and a slight improvement in code
readability.

Bill


Re: cvs commit: httpd-2.0/server/mpm/worker worker.c

Posted by Rodent of Unusual Size <Ke...@Golux.Com>.
Bill Stoddard wrote:
> 
> It's obvious we are not using the return either way.

But without the cast, someone not conversant with the details
won't know it normally returns a value; they might think it's
a void function.

> Less is better so dump the cast :-)

I think the cast improves understanding, and it doesn't
make any difference at the object-code level, so I say 'keep
it.' :-)
-- 
#ken	P-)}

Ken Coar, Sanagendamgagwedweinini  http://Golux.Com/coar/
Author, developer, opinionist      http://Apache-Server.Com/

"All right everyone!  Step away from the glowing hamburger!"

Re: cvs commit: httpd-2.0/server/mpm/worker worker.c

Posted by Bill Stoddard <bi...@wstoddard.com>.
> On Wed, Sep 19, 2001 at 06:34:11AM -0000, jwoolley@apache.org wrote:
> > jwoolley    01/09/18 23:34:11
> > 
> >   Modified:    server/mpm/worker worker.c
> >   Log:
> >   I was kinda hoping those (void)some_function() and (request_rec *)NULL
> >   casts would go away before this committed, but alas I didn't say anything.
> >   :-)  This gets rid of them and a few others just like them that I also
> >   found in worker.c.
> 
> I don't know what the groupthink is on this, or if there is a precedent
> set, but I think the (void)s make it obvious to the reader that we
> are deliberately throwing away the return value.
> 
> As for the (request_rec *)NULL thing, either way is fine for me.
> 
> -aaron

It's obvious we are not using the return either way. Less is better so dump the cast :-)

Bill