2.4.20-pre5: prevent sparc32's atomic_read() from possibly discarding
const qualifiers from pointers passed as its argument.
diff -urN linux-2.4.20-pre5/include/asm-sparc/atomic.h linux-2.4.20-pre5.n/include/asm-sparc/atomic.h
--- linux-2.4.20-pre5/include/asm-sparc/atomic.h 2001-11-08 17:42:19.000000000 +0100
+++ linux-2.4.20-pre5.n/include/asm-sparc/atomic.h 2002-09-01 12:29:36.000000000 +0200
@@ -35,7 +35,7 @@
#define ATOMIC_INIT(i) { (i << 8) }
-static __inline__ int atomic_read(atomic_t *v)
+static __inline__ int atomic_read(const atomic_t *v)
{
int ret = v->counter;
From: Tomas Szepe <[email protected]>
Date: Sun, 1 Sep 2002 12:56:43 +0200
2.4.20-pre5: prevent sparc32's atomic_read() from possibly discarding
const qualifiers from pointers passed as its argument.
-static __inline__ int atomic_read(atomic_t *v)
+static __inline__ int atomic_read(const atomic_t *v)
So the atomic_t is const is it? That's news to me.
I think you mean something like "atomic_t const * v" which means the
pointer is constant, not the value.
> From: Tomas Szepe <[email protected]>
> Date: Sun, 1 Sep 2002 12:56:43 +0200
>
> 2.4.20-pre5: prevent sparc32's atomic_read() from possibly discarding
> const qualifiers from pointers passed as its argument.
>
> -static __inline__ int atomic_read(atomic_t *v)
> +static __inline__ int atomic_read(const atomic_t *v)
>
> So the atomic_t is const is it? That's news to me.
>
> I think you mean something like "atomic_t const * v" which means the
> pointer is constant, not the value.
Precisely.
diff -u linux-2.4.20-pre5/include/asm-sparc/atomic.h linux-2.4.20-pre5.n/include/asm-sparc/atomic.h
--- linux-2.4.20-pre5/include/asm-sparc/atomic.h 2001-11-08 17:42:19.000000000 +0100
+++ linux-2.4.20-pre5.n/include/asm-sparc/atomic.h 2002-09-01 12:29:36.000000000 +0200
@@ -35,7 +35,7 @@
#define ATOMIC_INIT(i) { (i << 8) }
-static __inline__ int atomic_read(atomic_t *v)
+static __inline__ int atomic_read(atomic_t const *v)
{
int ret = v->counter;
From: Tomas Szepe <[email protected]>
Date: Sun, 1 Sep 2002 13:28:56 +0200
> I think you mean something like "atomic_t const * v" which means the
> pointer is constant, not the value.
Precisely.
BTW who even passes around const atomic_t's? Ie. what
genrated the warning and made you even edit this to begin with?
> From: Tomas Szepe <[email protected]>
> Date: Sun, 1 Sep 2002 13:28:56 +0200
>
> > I think you mean something like "atomic_t const * v" which means the
> > pointer is constant, not the value.
>
> Precisely.
>
> BTW who even passes around const atomic_t's? Ie. what
> genrated the warning and made you even edit this to begin with?
fs/reiserfs/buffer2.c, line ~28:
atomic_t gets the const quality on account of being a member
of a const struct buffer_head instance.
void wait_buffer_until_released (const struct buffer_head * bh)
{
int repeat_counter = 0;
while (atomic_read (&(bh->b_count)) > 1) {
if ( !(++repeat_counter % 30000000) ) {
reiserfs_warning ("vs-3050: wait_buffer_until_released: nobody releases buffer (%b). Still waiting (%d) %cJDIRTY %cJWAIT\n",
bh, repeat_counter, buffer_journaled(bh) ? ' ' : '!',
buffer_journal_dirty(bh) ? ' ' : '!');
}
run_task_queue(&tq_disk);
yield();
}
if (repeat_counter > 30000000) {
reiserfs_warning("vs-3051: done waiting, ignore vs-3050 messages for (%b)\n", bh) ;
}
}
From: Tomas Szepe <[email protected]>
Date: Sun, 1 Sep 2002 13:37:42 +0200
> BTW who even passes around const atomic_t's? Ie. what
> genrated the warning and made you even edit this to begin with?
fs/reiserfs/buffer2.c, line ~28:
atomic_t gets the const quality on account of being a member
of a const struct buffer_head instance.
void wait_buffer_until_released (const struct buffer_head * bh)
{
Reiserfs is buggy, it means struct buffer_head const * bh
Let's keep the sparc atomic_read() how it is so more bugs
like this can be found.
> Let's keep the sparc atomic_read() how it is so more bugs
> like this can be found.
I don't know, though... scratching my head here -- Is GCC actually
able to distinguish between 'const int *a' and 'int const *a'?
Because if 'int const *a' means that the pointer is constant but
not the actual value it points to,
void a(int const *a) { *a = 1; }
shouldn't be generating 'warning: assignment of read-only location'.
Right?
> Reiserfs is buggy, it means struct buffer_head const * bh
Okay so that gives us the third reiserfs patch of the day:
--- buffer2.c~ 2002-09-01 13:52:39.000000000 +0200
+++ buffer2.c 2002-09-01 13:44:19.000000000 +0200
@@ -21,7 +21,7 @@
hold we did free all buffers in tree balance structure
(get_empty_nodes and get_nodes_for_preserving) or in path structure
only (get_new_buffer) just before calling this */
-void wait_buffer_until_released (const struct buffer_head * bh)
+void wait_buffer_until_released (struct buffer_head const *bh)
{
int repeat_counter = 0;
From: Tomas Szepe <[email protected]>
Date: Sun, 1 Sep 2002 14:10:53 +0200
> Let's keep the sparc atomic_read() how it is so more bugs
> like this can be found.
I don't know, though... scratching my head here -- Is GCC actually
able to distinguish between 'const int *a' and 'int const *a'?
No I don't mean this in a "C" sense, I mean conceptually that marking
an object const which has members which are volatile and updated
asynchronously makes zero sense.
> From: Tomas Szepe <[email protected]>
> Date: Sun, 1 Sep 2002 14:10:53 +0200
>
> > Let's keep the sparc atomic_read() how it is so more bugs
> > like this can be found.
>
> I don't know, though... scratching my head here -- Is GCC actually
> able to distinguish between 'const int *a' and 'int const *a'?
>
> No I don't mean this in a "C" sense, I mean conceptually that marking
> an object const which has members which are volatile and updated
> asynchronously makes zero sense.
True.
I've been playing a bit with how gcc handles the const qualifiers
and made an interesting discovery:
Trying to compile
typedef int *p_int;
void a(const p_int t) { *t = 0; }
void b(const p_int t) { t = (int *) 0; }
void c(const int *t) { *t = 0; }
void d(const int *t) { t = (int *) 0; }
void e(int const *t) { *t = 0; }
void f(int const *t) { t = (int *) 0; }
will give 'assignment of read-only location' warnings for
b(), c() and e(), i.e. it's impossible to have a constant
pointer to a non-constant value w/o using a qualified
typedef.
W/o a typedef, gcc seems unable to tell the difference
between 'const int *' and 'int const *' altogether. In case
one needs a constant pointer to a constant value, something
like this should do:
typedef const int *p_int;
void f(const p_int a);
Usable? I don't quite think so.
T.
On Sun, 1 Sep 2002 14:39:03 +0200, Tomas Szepe wrote:
>I've been playing a bit with how gcc handles the const qualifiers
>and made an interesting discovery:
>
>Trying to compile
>
>typedef int *p_int;
>void a(const p_int t) { *t = 0; }
>void b(const p_int t) { t = (int *) 0; }
>void c(const int *t) { *t = 0; }
>void d(const int *t) { t = (int *) 0; }
>void e(int const *t) { *t = 0; }
>void f(int const *t) { t = (int *) 0; }
>
>will give 'assignment of read-only location' warnings for
>b(), c() and e(),
In b() t is a const value and you're trying to assign to it,
and in c() and e() t is a pointer-to-const and you're trying
to assign to *t. The compiler catches this. What's the problem?
>i.e. it's impossible to have a constant
>pointer to a non-constant value w/o using a qualified
>typedef.
void g(int * const t) { *t = 0; }
>W/o a typedef, gcc seems unable to tell the difference
>between 'const int *' and 'int const *' altogether.
There is no difference. Read the C spec, or Harbison&Steele
which has had an explanation of 'const' since their '87 2nd Ed.
/Mikael
> >i.e. it's impossible to have a constant
> >pointer to a non-constant value w/o using a qualified
> >typedef.
>
> void g(int * const t) { *t = 0; }
>
>
> >W/o a typedef, gcc seems unable to tell the difference
> >between 'const int *' and 'int const *' altogether.
>
> There is no difference. Read the C spec, or Harbison&Steele
> which has had an explanation of 'const' since their '87 2nd Ed.
Ok, that explains it, obviously I (and DaveM :D) didn't know the syntax.
Thanks for the reference!
I'm going to redo the patches.
T.
> 2.4.20-pre5: prevent sparc32's atomic_read() from possibly discarding
> const qualifiers from pointers passed as its argument.
Ok, the only reasonable way to deal with the last reiserfs vs.
sparc32 compilation warning is apparently to strip the const from
the wait_buffer_until_released() prototype, as it doesn't make any
sense there. Marcelo please disregard the atomic_read() patch and
apply the following instead:
diff -urN linux-2.4.20-pre5/fs/reiserfs/buffer2.c linux-2.4.20-pre5.n/fs/reiserfs/buffer2.c
--- linux-2.4.20-pre5/fs/reiserfs/buffer2.c 2002-09-01 17:19:26.000000000 +0200
+++ linux-2.4.20-pre5.n/fs/reiserfs/buffer2.c 2002-09-01 17:07:27.000000000 +0200
@@ -21,7 +21,7 @@
hold we did free all buffers in tree balance structure
(get_empty_nodes and get_nodes_for_preserving) or in path structure
only (get_new_buffer) just before calling this */
-void wait_buffer_until_released (const struct buffer_head * bh)
+void wait_buffer_until_released (struct buffer_head *bh)
{
int repeat_counter = 0;
diff -urN linux-2.4.20-pre5/include/linux/reiserfs_fs.h linux-2.4.20-pre5.n/include/linux/reiserfs_fs.h
--- linux-2.4.20-pre5/include/linux/reiserfs_fs.h 2002-09-01 17:19:27.000000000 +0200
+++ linux-2.4.20-pre5.n/include/linux/reiserfs_fs.h 2002-09-01 17:16:32.000000000 +0200
@@ -1845,7 +1847,7 @@
/* buffer2.c */
struct buffer_head * reiserfs_getblk (kdev_t n_dev, int n_block, int n_size);
-void wait_buffer_until_released (const struct buffer_head * bh);
+void wait_buffer_until_released (struct buffer_head *bh);
struct buffer_head * reiserfs_bread (struct super_block *super, int n_block,
int n_size);
On Sun, Sep 01, 2002 at 02:39:03PM +0200, Tomas Szepe wrote:
> I've been playing a bit with how gcc handles the const qualifiers
> and made an interesting discovery:
>
> Trying to compile
>
> typedef int *p_int;
> void a(const p_int t) { *t = 0; }
> void b(const p_int t) { t = (int *) 0; }
> void c(const int *t) { *t = 0; }
> void d(const int *t) { t = (int *) 0; }
> void e(int const *t) { *t = 0; }
> void f(int const *t) { t = (int *) 0; }
>
> will give 'assignment of read-only location' warnings for
> b(), c() and e(), i.e. it's impossible to have a constant
> pointer to a non-constant value w/o using a qualified
> typedef.
If you want a constant *pointer*, use:
void f(int* const t)
(read "f is a function, taking parameter constant pointer to
int, returning void)
> W/o a typedef, gcc seems unable to tell the difference
> between 'const int *' and 'int const *' altogether.
That's because there is no difference ("pointer to integer constant" vs
"pointer to constant integer").
See http://untroubled.org/articles/cdecls.txt for one of the best
references I've ever seen to understanding C type declarations.
--
Bruce Guenter <[email protected]> http://em.ca/~bruceg/ http://untroubled.org/
OpenPGP key: 699980E8 / D0B7 C8DD 365D A395 29DA 2E2A E96F B2DC 6999 80E8
On Sun, Sep 01, 2002 at 01:28:56PM +0200, Tomas Szepe wrote:
> > From: Tomas Szepe <[email protected]>
> > Date: Sun, 1 Sep 2002 12:56:43 +0200
> >
> > 2.4.20-pre5: prevent sparc32's atomic_read() from possibly discarding
> > const qualifiers from pointers passed as its argument.
> >
> > -static __inline__ int atomic_read(atomic_t *v)
> > +static __inline__ int atomic_read(const atomic_t *v)
> >
> > So the atomic_t is const is it? That's news to me.
> >
> > I think you mean something like "atomic_t const * v" which means the
> > pointer is constant, not the value.
>
> Precisely.
>
No, you don't. Having the pointer constant means the symbolic argument
can't be changed inside the function. But it means nothing at all to the
caller, because the caller's variable itself is never changed by the
call.
>
> diff -u linux-2.4.20-pre5/include/asm-sparc/atomic.h linux-2.4.20-pre5.n/include/asm-sparc/atomic.h
> --- linux-2.4.20-pre5/include/asm-sparc/atomic.h 2001-11-08 17:42:19.000000000 +0100
> +++ linux-2.4.20-pre5.n/include/asm-sparc/atomic.h 2002-09-01 12:29:36.000000000 +0200
> @@ -35,7 +35,7 @@
>
> #define ATOMIC_INIT(i) { (i << 8) }
>
> -static __inline__ int atomic_read(atomic_t *v)
> +static __inline__ int atomic_read(atomic_t const *v)
> {
> int ret = v->counter;
-------------------------------------------------------------------------------
Jan 'Bulb' Hudec <[email protected]>