[PATCH v2] rch/score/include/uapi/asm/ptrace.h: Add prefix 'SCORE_' for related macros

[PATCH v2] rch/score/include/uapi/asm/ptrace.h: Add prefix 'SCORE_' for related macros

[Chen Gang]

'COUNTER' and other same kind macros are too common to use, and easy to
get conflict with other modules. So add prefix for them.

And it is UAPI, so only change it within linux kernel.

The related warning (allmodconfig with score):

    CC [M]  drivers/md/raid1.o
  In file included from drivers/md/raid1.c:42:0:
  drivers/md/bitmap.h:93:0: warning: "COUNTER" redefined
   #define COUNTER(x) (((bitmap_counter_t) x) & COUNTER_MAX)
   ^
  In file included from ./arch/score/include/asm/ptrace.h:4:0,
                   from include/linux/sched.h:31,
                   from include/linux/blkdev.h:4,
                   from drivers/md/raid1.c:36:
  ./arch/score/include/uapi/asm/ptrace.h:13:0: note: this is the location of the previous definition
   #define COUNTER  38


Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
---
 arch/score/include/uapi/asm/ptrace.h | 33 +++++++++++++++++++++++----------
 1 file changed, 23 insertions(+), 10 deletions(-)

diff --git a/arch/score/include/uapi/asm/ptrace.h b/arch/score/include/uapi/asm/ptrace.h
index f59771a..56e2de0 100644
--- a/arch/score/include/uapi/asm/ptrace.h
+++ b/arch/score/include/uapi/asm/ptrace.h
@@ -4,16 +4,29 @@
 #define PTRACE_GETREGS		12
 #define PTRACE_SETREGS		13
 
-#define PC		32
-#define CONDITION	33
-#define ECR		34
-#define EMA		35
-#define CEH		36
-#define CEL		37
-#define COUNTER		38
-#define LDCR		39
-#define STCR		40
-#define PSR		41
+#if !defined(__KERNEL__) || !defined(__linux__)
+#define PC			32
+#define CONDITION		33
+#define ECR			34
+#define EMA			35
+#define CEH			36
+#define CEL			37
+#define COUNTER			38
+#define LDCR			39
+#define STCR			40
+#define PSR			41
+#else
+#define SCORE_PC		32
+#define SCORE_CONDITION		33
+#define SCORE_ECR		34
+#define SCORE_EMA		35
+#define SCORE_CEH		36
+#define SCORE_CEL		37
+#define SCORE_COUNTER		38
+#define SCORE_LDCR		39
+#define SCORE_STCR		40
+#define SCORE_PSR		41
+#endif
 
 #define SINGLESTEP16_INSN	0x7006
 #define SINGLESTEP32_INSN	0x840C8000
-- 
1.9.2.459.g68773ac

Re: [PATCH v2] rch/score/include/uapi/asm/ptrace.h: Add prefix 'SCORE_' for related macros

[Guenter Roeck]

On 06/24/2014 06:08 AM, Chen Gang wrote:
> 'COUNTER' and other same kind macros are too common to use, and easy to
> get conflict with other modules. So add prefix for them.
>
> And it is UAPI, so only change it within linux kernel.
>
> The related warning (allmodconfig with score):
>
>      CC [M]  drivers/md/raid1.o
>    In file included from drivers/md/raid1.c:42:0:
>    drivers/md/bitmap.h:93:0: warning: "COUNTER" redefined
>     #define COUNTER(x) (((bitmap_counter_t) x) & COUNTER_MAX)
>     ^
>    In file included from ./arch/score/include/asm/ptrace.h:4:0,
>                     from include/linux/sched.h:31,
>                     from include/linux/blkdev.h:4,
>                     from drivers/md/raid1.c:36:
>    ./arch/score/include/uapi/asm/ptrace.h:13:0: note: this is the location of the previous definition
>     #define COUNTER  38
>
>
> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
> ---
>   arch/score/include/uapi/asm/ptrace.h | 33 +++++++++++++++++++++++----------
>   1 file changed, 23 insertions(+), 10 deletions(-)
>
> diff --git a/arch/score/include/uapi/asm/ptrace.h b/arch/score/include/uapi/asm/ptrace.h
> index f59771a..56e2de0 100644
> --- a/arch/score/include/uapi/asm/ptrace.h
> +++ b/arch/score/include/uapi/asm/ptrace.h
> @@ -4,16 +4,29 @@
>   #define PTRACE_GETREGS		12
>   #define PTRACE_SETREGS		13
>
> -#define PC		32
> -#define CONDITION	33
> -#define ECR		34
> -#define EMA		35
> -#define CEH		36
> -#define CEL		37
> -#define COUNTER		38
> -#define LDCR		39
> -#define STCR		40
> -#define PSR		41
> +#if !defined(__KERNEL__) || !defined(__linux__)
> +#define PC			32
> +#define CONDITION		33
> +#define ECR			34
> +#define EMA			35
> +#define CEH			36
> +#define CEL			37
> +#define COUNTER			38
> +#define LDCR			39
> +#define STCR			40
> +#define PSR			41
> +#else
> +#define SCORE_PC		32
> +#define SCORE_CONDITION		33
> +#define SCORE_ECR		34
> +#define SCORE_EMA		35
> +#define SCORE_CEH		36
> +#define SCORE_CEL		37
> +#define SCORE_COUNTER		38
> +#define SCORE_LDCR		39
> +#define SCORE_STCR		40
> +#define SCORE_PSR		41
> +#endif
>
>   #define SINGLESTEP16_INSN	0x7006
>   #define SINGLESTEP32_INSN	0x840C8000
>
That looks weird ... not sure if that is a good solution either.

Are those defines actually used anywhere ? They don't seem to be used in the kernel.

Side note - an 'a' got missing in your headline.
And I'd suggest to abbreviate it to something like
	score/uapi: ptrace.h:

Thanks,
Guenter

Re: [PATCH v2] rch/score/include/uapi/asm/ptrace.h: Add prefix 'SCORE_' for related macros

[David Rientjes]

On Tue, 24 Jun 2014, Guenter Roeck wrote:

> > diff --git a/arch/score/include/uapi/asm/ptrace.h
> > b/arch/score/include/uapi/asm/ptrace.h
> > index f59771a..56e2de0 100644
> > --- a/arch/score/include/uapi/asm/ptrace.h
> > +++ b/arch/score/include/uapi/asm/ptrace.h
> > @@ -4,16 +4,29 @@
> >   #define PTRACE_GETREGS		12
> >   #define PTRACE_SETREGS		13
> > 
> > -#define PC		32
> > -#define CONDITION	33
> > -#define ECR		34
> > -#define EMA		35
> > -#define CEH		36
> > -#define CEL		37
> > -#define COUNTER		38
> > -#define LDCR		39
> > -#define STCR		40
> > -#define PSR		41
> > +#if !defined(__KERNEL__) || !defined(__linux__)
> > +#define PC			32
> > +#define CONDITION		33
> > +#define ECR			34
> > +#define EMA			35
> > +#define CEH			36
> > +#define CEL			37
> > +#define COUNTER			38
> > +#define LDCR			39
> > +#define STCR			40
> > +#define PSR			41
> > +#else
> > +#define SCORE_PC		32
> > +#define SCORE_CONDITION		33
> > +#define SCORE_ECR		34
> > +#define SCORE_EMA		35
> > +#define SCORE_CEH		36
> > +#define SCORE_CEL		37
> > +#define SCORE_COUNTER		38
> > +#define SCORE_LDCR		39
> > +#define SCORE_STCR		40
> > +#define SCORE_PSR		41
> > +#endif
> > 
> >   #define SINGLESTEP16_INSN	0x7006
> >   #define SINGLESTEP32_INSN	0x840C8000
> > 
> That looks weird ... not sure if that is a good solution either.
> 
> Are those defines actually used anywhere ? They don't seem to be used in the
> kernel.
> 
> Side note - an 'a' got missing in your headline.
> And I'd suggest to abbreviate it to something like
> 	score/uapi: ptrace.h:
> 

Besides checking whether they are used in the kernel or not, I presume we 
would need to be fairly confident there are actual userspace usecases for 
these that require the change.  It begs the question of why these 
constants are too generic to be in the kernel but are acceptable in the 
set of userspace applications in the world.

Re: [PATCH v2] rch/score/include/uapi/asm/ptrace.h: Add prefix 'SCORE_' for related macros

[Guenter Roeck]

On 06/24/2014 03:51 PM, David Rientjes wrote:
> On Tue, 24 Jun 2014, Guenter Roeck wrote:
>
>>> diff --git a/arch/score/include/uapi/asm/ptrace.h
>>> b/arch/score/include/uapi/asm/ptrace.h
>>> index f59771a..56e2de0 100644
>>> --- a/arch/score/include/uapi/asm/ptrace.h
>>> +++ b/arch/score/include/uapi/asm/ptrace.h
>>> @@ -4,16 +4,29 @@
>>>    #define PTRACE_GETREGS		12
>>>    #define PTRACE_SETREGS		13
>>>
>>> -#define PC		32
>>> -#define CONDITION	33
>>> -#define ECR		34
>>> -#define EMA		35
>>> -#define CEH		36
>>> -#define CEL		37
>>> -#define COUNTER		38
>>> -#define LDCR		39
>>> -#define STCR		40
>>> -#define PSR		41
>>> +#if !defined(__KERNEL__) || !defined(__linux__)
>>> +#define PC			32
>>> +#define CONDITION		33
>>> +#define ECR			34
>>> +#define EMA			35
>>> +#define CEH			36
>>> +#define CEL			37
>>> +#define COUNTER			38
>>> +#define LDCR			39
>>> +#define STCR			40
>>> +#define PSR			41
>>> +#else
>>> +#define SCORE_PC		32
>>> +#define SCORE_CONDITION		33
>>> +#define SCORE_ECR		34
>>> +#define SCORE_EMA		35
>>> +#define SCORE_CEH		36
>>> +#define SCORE_CEL		37
>>> +#define SCORE_COUNTER		38
>>> +#define SCORE_LDCR		39
>>> +#define SCORE_STCR		40
>>> +#define SCORE_PSR		41
>>> +#endif
>>>
>>>    #define SINGLESTEP16_INSN	0x7006
>>>    #define SINGLESTEP32_INSN	0x840C8000
>>>
>> That looks weird ... not sure if that is a good solution either.
>>
>> Are those defines actually used anywhere ? They don't seem to be used in the
>> kernel.
>>
>> Side note - an 'a' got missing in your headline.
>> And I'd suggest to abbreviate it to something like
>> 	score/uapi: ptrace.h:
>>
>
> Besides checking whether they are used in the kernel or not, I presume we
> would need to be fairly confident there are actual userspace usecases for
> these that require the change.  It begs the question of why these
> constants are too generic to be in the kernel but are acceptable in the
> set of userspace applications in the world.
>

That is another question. But for the time being it might be sufficient to
surround the defines with #if !defined(__KERNEL__) without introducing new
(and unused) kernel defines.

Guenter

Re: [PATCH v2] rch/score/include/uapi/asm/ptrace.h: Add prefix 'SCORE_' for related macros

[David Rientjes]

On Tue, 24 Jun 2014, Guenter Roeck wrote:

> That is another question. But for the time being it might be sufficient to
> surround the defines with #if !defined(__KERNEL__) without introducing new
> (and unused) kernel defines.
> 

On the other hand, if no userspace is referencing these, then you could 
just rename them and move them out of the uapi header to 
include/asm/ptrace.h and be done with it.

Re: [PATCH v2] rch/score/include/uapi/asm/ptrace.h: Add prefix 'SCORE_' for related macros

[Chen Gang]

On 06/25/2014 07:40 AM, David Rientjes wrote:
> On Tue, 24 Jun 2014, Guenter Roeck wrote:
> 
>> That is another question. But for the time being it might be sufficient to
>> surround the defines with #if !defined(__KERNEL__) without introducing new
>> (and unused) kernel defines.
>>
> 
> On the other hand, if no userspace is referencing these, then you could 
> just rename them and move them out of the uapi header to 
> include/asm/ptrace.h and be done with it.
> 

OK, thanks. And sorry for the subject lost 'a', firstly.  And it is
related with UAPI,  so it is really important enough to let related
maintainer to notice about it.

I guess, only the related maintainer know whether can remove these
macros out of "include/uapi/asm/ptrace.h" (although in current upstream
kernel source, they are really not used).

It is abnormal in uapi headers for same values with different macros
switched by "__KERNEL__", but if we have to remain them still in uapi,
we have to use "__KERNEL__ && __linux__" to carm down kernel and user.


Thanks.
-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed

Re: [PATCH v2] rch/score/include/uapi/asm/ptrace.h: Add prefix 'SCORE_' for related macros

[Lennox Wu]

These marcos are listed in advance.
Indeed, they have not been used, and the functions use them have not
be released.
I would like to keep the status even if the configure should not be used.
However, if guys think it is dirty, maybe we can remove or comment them first.

Best,
Lennox


2014-06-25 9:25 GMT+08:00 Chen Gang <gang.chen.5i5j@gmail.com>:
>
>
> On 06/25/2014 07:40 AM, David Rientjes wrote:
>> On Tue, 24 Jun 2014, Guenter Roeck wrote:
>>
>>> That is another question. But for the time being it might be sufficient to
>>> surround the defines with #if !defined(__KERNEL__) without introducing new
>>> (and unused) kernel defines.
>>>
>>
>> On the other hand, if no userspace is referencing these, then you could
>> just rename them and move them out of the uapi header to
>> include/asm/ptrace.h and be done with it.
>>
>
> OK, thanks. And sorry for the subject lost 'a', firstly.  And it is
> related with UAPI,  so it is really important enough to let related
> maintainer to notice about it.
>
> I guess, only the related maintainer know whether can remove these
> macros out of "include/uapi/asm/ptrace.h" (although in current upstream
> kernel source, they are really not used).
>
> It is abnormal in uapi headers for same values with different macros
> switched by "__KERNEL__", but if we have to remain them still in uapi,
> we have to use "__KERNEL__ && __linux__" to carm down kernel and user.
>
>
> Thanks.
> --
> Chen Gang
>
> Open, share, and attitude like air, water, and life which God blessed

Re: [PATCH v2] rch/score/include/uapi/asm/ptrace.h: Add prefix 'SCORE_' for related macros

[Chen Gang]

On 06/25/2014 11:16 PM, Lennox Wu wrote:
> These marcos are listed in advance.
> Indeed, they have not been used, and the functions use them have not
> be released.

UAPI is the main interface for contribution to outside, that is our
main goal, and has the highest priority.

After new contents are added into UAPI, it is very hard to remove them
again: it is protocol, can only be complement, but can not be changed
(or will be a 'cheat').


> I would like to keep the status even if the configure should not be used.
> However, if guys think it is dirty, maybe we can remove or comment them first.
> 

So, for me, if we find the new contents in UAPI may have negative effect to
outside, we need stop it firstly (at least, not let it merged into main line).


If no any additional reply for it within this week, I shall send patch
v3 for it (simply remove them).

Thanks.
-- 
Chen Gang

Open share and attitude like air water and life which God blessed

Re: [PATCH v2] rch/score/include/uapi/asm/ptrace.h: Add prefix 'SCORE_' for related macros

[Guenter Roeck]

On Thu, Jun 26, 2014 at 12:05:33AM +0000, Wu Yu-Chen wrote:
> As I said, these macros are useless currently. I don’t think users will use these macros since the kernel doesn’t respond these macros. However, you are welcome to provide your idea 😊
> 
Just drop them entirely.

Guenter