Re: svn commit: r330141 - /apr/apr-util/trunk/dbd/apr_dbd.c

[André Malo <nd...@perlig.de>]

* niq@apache.org wrote:

> +#define CLEANUP_CAST (apr_status_t (*)(void*))

Hmm. That looks strange to me. Wouldn't a typedef (+ casts in place) serve 
better?

nd
-- 
s  s^saaaaaoaaaoaaaaooooaaoaaaomaaaa  a  alataa  aaoat  a  a
a maoaa a laoata  a  oia a o  a m a  o  alaoooat aaool aaoaa
matooololaaatoto  aaa o a  o ms;s;\s;s;g;y;s;:;s;y#mailto: #
 \51/\134\137| http://www.perlig.de #;print;# > nd@perlig.de

Re: svn commit: r330141 - /apr/apr-util/trunk/dbd/apr_dbd.c

[Joe Orton <jo...@redhat.com>]

On Wed, Nov 02, 2005 at 11:07:21AM +0000, Nick Kew wrote:
> On Wednesday 02 November 2005 09:03, Joe Orton wrote:
> 
> > This code has undefined behaviour by the C standard (calling a function
> > via a function pointer of incompatible type), such casts should never be
> > used at all IMO.
> 
> So where is the incompatible pointer type?

(apr_status_t (*)(apr_thread_mutex_t *)) is not compatible with 
(apr_status_t (*)(void *)), likewise for the ->end_transaction callback.  
If it was compatible the cast wouldn't be needed.

joe

Re: svn commit: r330141 - /apr/apr-util/trunk/dbd/apr_dbd.c

[Nick Kew <ni...@webthing.com>]

On Wednesday 02 November 2005 09:03, Joe Orton wrote:

> This code has undefined behaviour by the C standard (calling a function
> via a function pointer of incompatible type), such casts should never be
> used at all IMO.

So where is the incompatible pointer type?

-- 
Nick Kew

Re: svn commit: r330141 - /apr/apr-util/trunk/dbd/apr_dbd.c

[Joe Orton <jo...@redhat.com>]

On Tue, Nov 01, 2005 at 11:54:18PM +0100, André Malo wrote:
> * Nick Kew wrote:
> 
> > On Tuesday 01 November 2005 22:44, André Malo wrote:
> > > * niq@apache.org wrote:
> > > > +#define CLEANUP_CAST (apr_status_t (*)(void*))
> > >
> > > Hmm. That looks strange to me. Wouldn't a typedef (+ casts in place)
> > > serve better?
> >
> > Huh?  It is a cast.  No more, no less.
> 
> Yep. I'd do it in place anyway. The macro actually decreases readability 
> here (IMHO, of course ;-).
> 
> Further thinking, the typedef is actually missing in apr_pools.h. Not sure 
> about the policy - could this be added for the next APR version?

This code has undefined behaviour by the C standard (calling a function 
via a function pointer of incompatible type), such casts should never be 
used at all IMO.

joe

Re: svn commit: r330141 - /apr/apr-util/trunk/dbd/apr_dbd.c

[André Malo <nd...@perlig.de>]

* Nick Kew wrote:

> On Tuesday 01 November 2005 22:44, André Malo wrote:
> > * niq@apache.org wrote:
> > > +#define CLEANUP_CAST (apr_status_t (*)(void*))
> >
> > Hmm. That looks strange to me. Wouldn't a typedef (+ casts in place)
> > serve better?
>
> Huh?  It is a cast.  No more, no less.

Yep. I'd do it in place anyway. The macro actually decreases readability 
here (IMHO, of course ;-).

Further thinking, the typedef is actually missing in apr_pools.h. Not sure 
about the policy - could this be added for the next APR version?

nd
-- 
"Das Verhalten von Gates hatte mir bewiesen, dass ich auf ihn und seine
beiden Gefährten nicht zu zählen brauchte" -- Karl May, "Winnetou III"

Im Westen was neues: <http://pub.perlig.de/books.html#apache2>

Re: svn commit: r330141 - /apr/apr-util/trunk/dbd/apr_dbd.c

[Nick Kew <ni...@webthing.com>]

On Tuesday 01 November 2005 22:44, André Malo wrote:
> * niq@apache.org wrote:
> > +#define CLEANUP_CAST (apr_status_t (*)(void*))
>
> Hmm. That looks strange to me. Wouldn't a typedef (+ casts in place) serve
> better?

Huh?  It is a cast.  No more, no less.

-- 
Nick Kew

Re: svn commit: r330141 - /apr/apr-util/trunk/dbd/apr_dbd.c

[Garrett Rooney <ro...@electricjellyfish.net>]

On 11/1/05, André Malo <nd...@perlig.de> wrote:
> * niq@apache.org wrote:
>
> > +#define CLEANUP_CAST (apr_status_t (*)(void*))
>
> Hmm. That looks strange to me. Wouldn't a typedef (+ casts in place) serve
> better?

+1, I was actually thinking the same thing.

-garrett