get_new_cred clearly writes through the pointer, so const isn't
appropriate. Sparse warns thusly:
include/linux/cred.h: In function ‘get_cred’:
include/linux/cred.h:181: warning: passing argument 1 of ‘get_new_cred’ discards qualifiers from pointer target type
Signed-off-by: Harvey Harrison <[email protected]>
---
include/linux/cred.h | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/cred.h b/include/linux/cred.h
index b156ed4..174eb56 100644
--- a/include/linux/cred.h
+++ b/include/linux/cred.h
@@ -176,9 +176,9 @@ static inline struct cred *get_new_cred(struct cred *cred)
* Get a reference on the specified set of credentials. The caller must
* release the reference.
*/
-static inline const struct cred *get_cred(const struct cred *cred)
+static inline struct cred *get_cred(struct cred *cred)
{
- return get_new_cred((struct cred *) cred);
+ return get_new_cred(cred);
}
/**
--
1.6.0.rc1.278.g9c632
Harvey Harrison <[email protected]> wrote:
> get_new_cred clearly writes through the pointer, so const isn't
> appropriate. Sparse warns thusly:
>
> include/linux/cred.h: In function ?get_cred?:
> include/linux/cred.h:181: warning: passing argument 1 of ?get_new_cred? discards qualifiers from pointer target type
Sparse is wrong in this instance, it failed to note the cast. I know what I'm
doing.
> -static inline const struct cred *get_cred(const struct cred *cred)
> +static inline struct cred *get_cred(struct cred *cred)
That will break the compilation. Please don't do that.
The point of my use of const in this instance is to stop people from trying to
modify committed credentials directly, especially current->cred. But we still
have to be able to take a reference to it. Unfortunately, C does not provide
the necessary tools to do what I want.
Eventually, we can probably ditch the const marks on the pointers but not yet.
David
On Sun, 10 Aug 2008 11:58:36 +0100 David Howells <[email protected]> wrote:
> Harvey Harrison <[email protected]> wrote:
>
> > get_new_cred clearly writes through the pointer, so const isn't
> > appropriate. Sparse warns thusly:
> >
> > include/linux/cred.h: In function ___get_cred___:
> > include/linux/cred.h:181: warning: passing argument 1 of ___get_new_cred___ discards qualifiers from pointer target type
>
> Sparse is wrong in this instance, it failed to note the cast. I know what I'm
> doing.
Nobody who reads the code will know what you were doing - the code
looks plain wrong.
> > -static inline const struct cred *get_cred(const struct cred *cred)
> > +static inline struct cred *get_cred(struct cred *cred)
>
> That will break the compilation. Please don't do that.
>
> The point of my use of const in this instance is to stop people from trying to
> modify committed credentials directly, especially current->cred. But we still
> have to be able to take a reference to it. Unfortunately, C does not provide
> the necessary tools to do what I want.
>
> Eventually, we can probably ditch the const marks on the pointers but not yet.
That information should have been included in a code comment.
Andrew Morton <[email protected]> wrote:
> That information should have been included in a code comment.
It was explained in the covering note:
Subject: [PATCH 00/24] Introduce credentials [ver #7]
...
(c) task_struct::cred is a const struct cred *, as are all pointers
that aren't used specifically for creating new credentials. This
catches places that are changing creds when they shouldn't be at
compile time.
To get a new ref on a const cred, use get_cred() which casts away
the const and calls atomic_inc().
It should've gone in the log entry for the "CRED: Inaugurate COW credentials"
patch and also into the documentation. I don't know whether I can amend the
former, but I can amend the latter. I'll also amend the comment to make it
crystal clear.
David