You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Peter Samuelson <pe...@p12n.org> on 2006/01/18 10:27:23 UTC
Re: svn commit: r18144 - in trunk/subversion/libsvn_fs_base: . bdb
[brane@tigris.org]
> -svn_fs_bdb__dberrf (bdb_errcall_baton_t *ec_baton,
> - int db_err, const char *fmt, ...)
> +svn_fs_bdb__dberrf (bdb_env_baton_t *bdb_baton,
> + int db_err, const char *fmt, ...)
A few ^I slipped in at some point. I wouldn't have noticed except that
it's aligned for 4-space tabs, not 8-space tabs.
Re: svn commit: r18144 - in trunk/subversion/libsvn_fs_base: . bdb
Posted by Malcolm Rowe <ma...@farside.org.uk>.
On Sun, Jan 22, 2006 at 07:39:47PM +0100, Branko Čibej wrote:
> Malcolm Rowe wrote:
> >You'd defined bdb_cache_state as 'volatile void *', and so you're passing
> >a 'volatile void **' to apr_atomic_casptr(), for which the default
> >(macro) implementation of casts its first argument to (apr_uint32_t *)
> >- see include/apr_atomic.h:285. Isn't that a type-punned pointer?
> >
> The prototype for apr_atomic_casptr says that the first parameter is
> volatile void **. If there's a macro doing a cast getting in between,
> then that's a bug in APR and should be reported there.
>
Sorry, I should have been clearer. The default macro implementation of
apr_atomic_casptr (used on everything but Windows and Netware, it seems)
is implemented in terms of apr_atomic_cas, which _does_ take an
apr_uint32_t *:
#define apr_atomic_casptr(mem, with, cmp) \
(void *)apr_atomic_cas((apr_uint32_t *)(mem), (long)(with), (long)cmp)
So it might be correct, but I guess it's still a type-punned pointer.
Regards,
Malcolm
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r18144 - in trunk/subversion/libsvn_fs_base: . bdb
Posted by Philip Martin <ph...@codematters.co.uk>.
Branko Čibej <br...@xbc.nu> writes:
> Malcolm Rowe wrote:
>>
>> You'd defined bdb_cache_state as 'volatile void *', and so you're passing
>> a 'volatile void **' to apr_atomic_casptr(), for which the default
>> (macro) implementation of casts its first argument to (apr_uint32_t *)
>> - see include/apr_atomic.h:285. Isn't that a type-punned pointer?
Malcolm is describing the 0.9.x version of apr_atomic.h which includes
a (Linux only?) macro implementation in the public header file
> The prototype for apr_atomic_casptr says that the first parameter is
> volatile void **. If there's a macro doing a cast getting in between,
> then that's a bug in APR and should be reported there.
while Branko is describing 1.2.x which no longer includes the macro.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r18144 - in trunk/subversion/libsvn_fs_base: . bdb
Posted by Branko Čibej <br...@xbc.nu>.
Malcolm Rowe wrote:
> On Wed, Jan 18, 2006 at 10:12:26PM +0100, Branko Čibej wrote:
>
>> Philip Martin wrote:
>>
>>> Malcolm Rowe <ma...@farside.org.uk> writes:
>>>
>>>
>>>
>>>> All four apr_atomic_casptr() calls in bdb/env.c generate the following
>>>> warning for me, breaking my '-Wall -Werror' build (gcc 3.4.4 on RHEL4):
>>>>
>>>> ../svn-live-trunk-srcdir/subversion/libsvn_fs_base/bdb/env.c: In function
>>>> `bdb_cache_init':
>>>> ../svn-live-trunk-srcdir/subversion/libsvn_fs_base/bdb/env.c:340:
>>>> warning: dereferencing type-punned pointer will break strict-aliasing
>>>> rules
>>>>
>>>>
>>> I'm using gcc 4.0.3 and I don't get that warning.
>>>
>>>
>> Yes, I can't see any type-punned pointers there.
>>
>>
>
> You'd defined bdb_cache_state as 'volatile void *', and so you're passing
> a 'volatile void **' to apr_atomic_casptr(), for which the default
> (macro) implementation of casts its first argument to (apr_uint32_t *)
> - see include/apr_atomic.h:285. Isn't that a type-punned pointer?
>
The prototype for apr_atomic_casptr says that the first parameter is
volatile void **. If there's a macro doing a cast getting in between,
then that's a bug in APR and should be reported there.
> In any case, you fixed it in r18170, so thanks.
>
Not me. :) It was maxb.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r18144 - in trunk/subversion/libsvn_fs_base: . bdb
Posted by Malcolm Rowe <ma...@farside.org.uk>.
On Wed, Jan 18, 2006 at 10:12:26PM +0100, Branko Čibej wrote:
> Philip Martin wrote:
> >Malcolm Rowe <ma...@farside.org.uk> writes:
> >
> >
> >>All four apr_atomic_casptr() calls in bdb/env.c generate the following
> >>warning for me, breaking my '-Wall -Werror' build (gcc 3.4.4 on RHEL4):
> >>
> >>../svn-live-trunk-srcdir/subversion/libsvn_fs_base/bdb/env.c: In function
> >>`bdb_cache_init':
> >>../svn-live-trunk-srcdir/subversion/libsvn_fs_base/bdb/env.c:340:
> >>warning: dereferencing type-punned pointer will break strict-aliasing
> >>rules
> >>
> >
> >I'm using gcc 4.0.3 and I don't get that warning.
> >
> Yes, I can't see any type-punned pointers there.
>
You'd defined bdb_cache_state as 'volatile void *', and so you're passing
a 'volatile void **' to apr_atomic_casptr(), for which the default
(macro) implementation of casts its first argument to (apr_uint32_t *)
- see include/apr_atomic.h:285. Isn't that a type-punned pointer?
In any case, you fixed it in r18170, so thanks.
Regards,
Malcolm
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r18144 - in trunk/subversion/libsvn_fs_base: . bdb
Posted by Branko Čibej <br...@xbc.nu>.
Philip Martin wrote:
> Malcolm Rowe <ma...@farside.org.uk> writes:
>
>
>> All four apr_atomic_casptr() calls in bdb/env.c generate the following
>> warning for me, breaking my '-Wall -Werror' build (gcc 3.4.4 on RHEL4):
>>
>> ../svn-live-trunk-srcdir/subversion/libsvn_fs_base/bdb/env.c: In function `bdb_cache_init':
>> ../svn-live-trunk-srcdir/subversion/libsvn_fs_base/bdb/env.c:340: warning: dereferencing type-punned pointer will break strict-aliasing rules
>>
>
> I'm using gcc 4.0.3 and I don't get that warning.
>
Yes, I can't see any type-punned pointers there.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r18144 - in trunk/subversion/libsvn_fs_base: . bdb
Posted by Philip Martin <ph...@codematters.co.uk>.
Malcolm Rowe <ma...@farside.org.uk> writes:
> All four apr_atomic_casptr() calls in bdb/env.c generate the following
> warning for me, breaking my '-Wall -Werror' build (gcc 3.4.4 on RHEL4):
>
> ../svn-live-trunk-srcdir/subversion/libsvn_fs_base/bdb/env.c: In function `bdb_cache_init':
> ../svn-live-trunk-srcdir/subversion/libsvn_fs_base/bdb/env.c:340: warning: dereferencing type-punned pointer will break strict-aliasing rules
I'm using gcc 4.0.3 and I don't get that warning.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r18144 - in trunk/subversion/libsvn_fs_base: . bdb
Posted by Malcolm Rowe <ma...@farside.org.uk>.
[brane@tigris.org]
> void *cache_state = apr_atomic_casptr(&bdb_cache_state,
> bdb_cache_start_init, NULL);
Hi Branko,
All four apr_atomic_casptr() calls in bdb/env.c generate the following
warning for me, breaking my '-Wall -Werror' build (gcc 3.4.4 on RHEL4):
../svn-live-trunk-srcdir/subversion/libsvn_fs_base/bdb/env.c: In function `bdb_cache_init':
../svn-live-trunk-srcdir/subversion/libsvn_fs_base/bdb/env.c:340: warning: dereferencing type-punned pointer will break strict-aliasing rules
../svn-live-trunk-srcdir/subversion/libsvn_fs_base/bdb/env.c:357: warning: dereferencing type-punned pointer will break strict-aliasing rules
../svn-live-trunk-srcdir/subversion/libsvn_fs_base/bdb/env.c:365: warning: dereferencing type-punned pointer will break strict-aliasing rules
../svn-live-trunk-srcdir/subversion/libsvn_fs_base/bdb/env.c:382: warning: dereferencing type-punned pointer will break strict-aliasing rules
make: *** [subversion/libsvn_fs_base/bdb/env.lo] Error 1
I'm not sure how best to fix this: the type required seems to be
platform-specific.
Regards,
Malcolm
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r18144 - in trunk/subversion/libsvn_fs_base: . bdb
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
--On January 18, 2006 4:06:31 PM -0800 Daniel Rall <dl...@collab.net> wrote:
> The JavaHL code is riddled with them. :-\
>
> Shall I get rid of them?
+1. -- justin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r18144 - in trunk/subversion/libsvn_fs_base: . bdb
Posted by Branko Čibej <br...@xbc.nu>.
Daniel Rall wrote:
> On Wed, 18 Jan 2006, Branko Čibej wrote:
>
>
>> Branko Čibej wrote:
>>
>>> Peter Samuelson wrote:
>>>
>>>> [brane@tigris.org]
>>>>
>>>>
>>>>> -svn_fs_bdb__dberrf (bdb_errcall_baton_t *ec_baton,
>>>>> - int db_err, const char *fmt, ...)
>>>>> +svn_fs_bdb__dberrf (bdb_env_baton_t *bdb_baton,
>>>>> + int db_err, const char *fmt, ...)
>>>>>
>>>>>
>>>> A few ^I slipped in at some point. I wouldn't have noticed except that
>>>> it's aligned for 4-space tabs, not 8-space tabs.
>>>>
>>>>
>>> Ouch! Thanks for noticing this; I can't imagine how I did this, since
>>> I use an editor that's smart enough to not insert tabs into source
>>> files... I'll fix it.
>>>
>> Heh, I found 6 tabs in the BDB back-end, five of which weren't my fault. :)
>> Fixed in r18164.
>>
>> So I did a full search thought our code, and found lots and lots of
>> tabs. JavaHL/native has most of them.
>>
>
> The JavaHL code is riddled with them. :-\
>
> Shall I get rid of them?
>
Just as you like. Speaking formally, "yes of course you have to;" but
from a practical standpoint, it's just noise on the commits list.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r18144 - in trunk/subversion/libsvn_fs_base: . bdb
Posted by Daniel Rall <dl...@collab.net>.
On Wed, 18 Jan 2006, Branko Čibej wrote:
> Branko Čibej wrote:
> >Peter Samuelson wrote:
> >>[brane@tigris.org]
> >>
> >>>-svn_fs_bdb__dberrf (bdb_errcall_baton_t *ec_baton,
> >>>- int db_err, const char *fmt, ...)
> >>>+svn_fs_bdb__dberrf (bdb_env_baton_t *bdb_baton,
> >>>+ int db_err, const char *fmt, ...)
> >>>
> >>
> >>A few ^I slipped in at some point. I wouldn't have noticed except that
> >>it's aligned for 4-space tabs, not 8-space tabs.
> >>
> >Ouch! Thanks for noticing this; I can't imagine how I did this, since
> >I use an editor that's smart enough to not insert tabs into source
> >files... I'll fix it.
> Heh, I found 6 tabs in the BDB back-end, five of which weren't my fault. :)
> Fixed in r18164.
>
> So I did a full search thought our code, and found lots and lots of
> tabs. JavaHL/native has most of them.
The JavaHL code is riddled with them. :-\
Shall I get rid of them?
--
Daniel Rall
Re: svn commit: r18144 - in trunk/subversion/libsvn_fs_base: . bdb
Posted by Branko Čibej <br...@xbc.nu>.
Branko Čibej wrote:
> Peter Samuelson wrote:
>> [brane@tigris.org]
>>
>>> -svn_fs_bdb__dberrf (bdb_errcall_baton_t *ec_baton,
>>> - int db_err, const char *fmt, ...)
>>> +svn_fs_bdb__dberrf (bdb_env_baton_t *bdb_baton,
>>> + int db_err, const char *fmt, ...)
>>>
>>
>> A few ^I slipped in at some point. I wouldn't have noticed except that
>> it's aligned for 4-space tabs, not 8-space tabs.
>>
> Ouch! Thanks for noticing this; I can't imagine how I did this, since
> I use an editor that's smart enough to not insert tabs into source
> files... I'll fix it.
Heh, I found 6 tabs in the BDB back-end, five of which weren't my fault. :)
Fixed in r18164.
So I did a full search thought our code, and found lots and lots of
tabs. JavaHL/native has most of them.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r18144 - in trunk/subversion/libsvn_fs_base: . bdb
Posted by Branko Čibej <br...@xbc.nu>.
Peter Samuelson wrote:
> [brane@tigris.org]
>
>> -svn_fs_bdb__dberrf (bdb_errcall_baton_t *ec_baton,
>> - int db_err, const char *fmt, ...)
>> +svn_fs_bdb__dberrf (bdb_env_baton_t *bdb_baton,
>> + int db_err, const char *fmt, ...)
>>
>
> A few ^I slipped in at some point. I wouldn't have noticed except that
> it's aligned for 4-space tabs, not 8-space tabs.
>
Ouch! Thanks for noticing this; I can't imagine how I did this, since I
use an editor that's smart enough to not insert tabs into source
files... I'll fix it.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org