Quoting Will Drewry ([email protected]):
> Replaces the seccomp_t typedef with seccomp_struct to match modern
> kernel style.
(sorry, I'm a bit behind on list)
You were going to switch this to 'struct seccomp' right?
> Signed-off-by: Will Drewry <[email protected]>
> ---
> include/linux/sched.h | 2 +-
> include/linux/seccomp.h | 10 ++++++----
> 2 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 4032ec1..288b5cb 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -1418,7 +1418,7 @@ struct task_struct {
> uid_t loginuid;
> unsigned int sessionid;
> #endif
> - seccomp_t seccomp;
> + struct seccomp_struct seccomp;
>
> /* Thread group tracking */
> u32 parent_exec_id;
> diff --git a/include/linux/seccomp.h b/include/linux/seccomp.h
> index cc7a4e9..171ab66 100644
> --- a/include/linux/seccomp.h
> +++ b/include/linux/seccomp.h
> @@ -7,7 +7,9 @@
> #include <linux/thread_info.h>
> #include <asm/seccomp.h>
>
> -typedef struct { int mode; } seccomp_t;
> +struct seccomp_struct {
> + int mode;
> +};
>
> extern void __secure_computing(int);
> static inline void secure_computing(int this_syscall)
> @@ -19,7 +21,7 @@ static inline void secure_computing(int this_syscall)
> extern long prctl_get_seccomp(void);
> extern long prctl_set_seccomp(unsigned long);
>
> -static inline int seccomp_mode(seccomp_t *s)
> +static inline int seccomp_mode(struct seccomp_struct *s)
> {
> return s->mode;
> }
> @@ -28,7 +30,7 @@ static inline int seccomp_mode(seccomp_t *s)
>
> #include <linux/errno.h>
>
> -typedef struct { } seccomp_t;
> +struct seccomp_struct { };
>
> #define secure_computing(x) do { } while (0)
>
> @@ -42,7 +44,7 @@ static inline long prctl_set_seccomp(unsigned long arg2)
> return -EINVAL;
> }
>
> -static inline int seccomp_mode(seccomp_t *s)
> +static inline int seccomp_mode(struct seccomp_struct *s)
> {
> return 0;
> }
> --
> 1.7.5.4
>
On Thu, Feb 2, 2012 at 7:29 AM, Serge E. Hallyn
<[email protected]> wrote:
> Quoting Will Drewry ([email protected]):
>> Replaces the seccomp_t typedef with seccomp_struct to match modern
>> kernel style.
>
> (sorry, I'm a bit behind on list)
>
> You were going to switch this to 'struct seccomp' right?
I wasn;'t sure if
task_struct {
...
struct seccomp seccomp;
}
was as ideal. I've noticed that almost all of the duplicate names in
the task struct use redundancy to differentiate the naming, but I'm
happy enough to rename if appropriate.
>> Signed-off-by: Will Drewry <[email protected]>
>> ---
>> ?include/linux/sched.h ? | ? ?2 +-
>> ?include/linux/seccomp.h | ? 10 ++++++----
>> ?2 files changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/linux/sched.h b/include/linux/sched.h
>> index 4032ec1..288b5cb 100644
>> --- a/include/linux/sched.h
>> +++ b/include/linux/sched.h
>> @@ -1418,7 +1418,7 @@ struct task_struct {
>> ? ? ? uid_t loginuid;
>> ? ? ? unsigned int sessionid;
>> ?#endif
>> - ? ? seccomp_t seccomp;
>> + ? ? struct seccomp_struct seccomp;
>>
>> ?/* Thread group tracking */
>> ? ? ? u32 parent_exec_id;
>> diff --git a/include/linux/seccomp.h b/include/linux/seccomp.h
>> index cc7a4e9..171ab66 100644
>> --- a/include/linux/seccomp.h
>> +++ b/include/linux/seccomp.h
>> @@ -7,7 +7,9 @@
>> ?#include <linux/thread_info.h>
>> ?#include <asm/seccomp.h>
>>
>> -typedef struct { int mode; } seccomp_t;
>> +struct seccomp_struct {
>> + ? ? int mode;
>> +};
>>
>> ?extern void __secure_computing(int);
>> ?static inline void secure_computing(int this_syscall)
>> @@ -19,7 +21,7 @@ static inline void secure_computing(int this_syscall)
>> ?extern long prctl_get_seccomp(void);
>> ?extern long prctl_set_seccomp(unsigned long);
>>
>> -static inline int seccomp_mode(seccomp_t *s)
>> +static inline int seccomp_mode(struct seccomp_struct *s)
>> ?{
>> ? ? ? return s->mode;
>> ?}
>> @@ -28,7 +30,7 @@ static inline int seccomp_mode(seccomp_t *s)
>>
>> ?#include <linux/errno.h>
>>
>> -typedef struct { } seccomp_t;
>> +struct seccomp_struct { };
>>
>> ?#define secure_computing(x) do { } while (0)
>>
>> @@ -42,7 +44,7 @@ static inline long prctl_set_seccomp(unsigned long arg2)
>> ? ? ? return -EINVAL;
>> ?}
>>
>> -static inline int seccomp_mode(seccomp_t *s)
>> +static inline int seccomp_mode(struct seccomp_struct *s)
>> ?{
>> ? ? ? return 0;
>> ?}
>> --
>> 1.7.5.4
>>
On Fri, Feb 3, 2012 at 3:16 PM, Will Drewry <[email protected]> wrote:
>
> task_struct {
> ?...
> ?struct seccomp seccomp;
> }
>
> was as ideal. ?I've noticed that almost all of the duplicate names in
> the task struct use redundancy to differentiate the naming, but I'm
> happy enough to rename if appropriate.
The redundant "struct xyz_struct" naming is traditional, but we try to
avoid it these days. The reason for it is that I long long ago was a
bit confused about the C namespace rules, so for the longest time I
made struct names unique for no really good reason. The struct/union
namespace is separate from the other namespaces, so trying to make
things unique really has no good reason.
And obviously "struct task_struct" is one of those very old things,
and then the "struct xyz_struct" naming kind of spread from there.
I think "struct seccomp" is fine, and even if "struct x x" looks a bit
odd, it's at least _less_ repetition than "struct x_struct x" which is
just really repetitive.
That said, just to make "grep" easier, please do the whole "struct
xyz" always together, and always with just a single space in between
them, so that
git grep "struct xyz"
does the right thing. And for the same reason, when declaring a
struct, people should always use "struct xyz {", with that exact
spacing. The exact details of spacing obviously has no semantic
meaning, but making it easy to grep for use and for definition is
really convenient.
Linus
On Fri, Feb 3, 2012 at 7:05 PM, Linus Torvalds
<[email protected]> wrote:
> On Fri, Feb 3, 2012 at 3:16 PM, Will Drewry <[email protected]> wrote:
>>
>> task_struct {
>> ?...
>> ?struct seccomp seccomp;
>> }
>>
>> was as ideal. ?I've noticed that almost all of the duplicate names in
>> the task struct use redundancy to differentiate the naming, but I'm
>> happy enough to rename if appropriate.
>
> The redundant "struct xyz_struct" naming is traditional, but we try to
> avoid it these days. The reason for it is that I long long ago was a
> bit confused about the C namespace rules, so for the longest time I
> made struct names unique for no really good reason. The struct/union
> namespace is separate from the other namespaces, so trying to make
> things unique really has no good reason.
>
> And obviously "struct task_struct" is one of those very old things,
> and then the "struct xyz_struct" naming kind of spread from there.
>
> I think "struct seccomp" is fine, and even if "struct x x" looks a bit
> odd, it's at least _less_ repetition than "struct x_struct x" which is
> just really repetitive.
>
> That said, just to make "grep" easier, please do the whole "struct
> xyz" always together, and always with just a single space in between
> them, so that
>
> ? git grep "struct xyz"
>
> does the right thing. And for the same reason, when declaring a
> struct, people should always use "struct xyz {", with that exact
> spacing. The exact details of spacing obviously has no semantic
> meaning, but making it easy to grep for use and for definition is
> really convenient.
Thanks for the background and explanation!
will