Re: [PATCH 2/3] bpf: Remove dead variable

[Daniel Borkmann]

On 10/16/2017 09:22 PM, Richard Weinberger wrote:
[...]

So, I can happily squash 2/3 into 1/3 and resent.


Yeah, please just squash them.

Thanks,
Daniel

Re: [PATCH 2/3] bpf: Remove dead variable

[Daniel Borkmann]

On 10/16/2017 09:22 PM, Richard Weinberger wrote:
[...]

So, I can happily squash 2/3 into 1/3 and resent.


Yeah, please just squash them.

Thanks,
Daniel

Re: [PATCH 2/3] bpf: Remove dead variable

[Richard Weinberger]

Am Montag, 16. Oktober 2017, 21:11:47 CEST schrieb Daniel Borkmann: 
> > I can squash it into 1/3, I kept it that way because
> > even without 1/3 this variable is unused.
> 
> Hmm, the helper looks like the below. In patch 1/3 you removed
> the 'if (unlikely(!task))' test where the variable was used before,
> so 2/3 without the 1/3 would result in a compile error.

Why a compile error? It emits a warning.

> BPF_CALL_0(bpf_get_current_uid_gid)
> {
>   struct task_struct *task = current;
>   kuid_t uid;
>   kgid_t gid;
> 
>   if (unlikely(!task))
>   return -EINVAL;

Well, this is the only "user". Okay.

>   current_uid_gid(, );

Here we use current. So, task was always in vain.

So, I can happily squash 2/3 into 1/3 and resent.
The series just represented the way I've worked on the code... 

Thanks,
//richard

Re: [PATCH 2/3] bpf: Remove dead variable

[Richard Weinberger]

Am Montag, 16. Oktober 2017, 21:11:47 CEST schrieb Daniel Borkmann: 
> > I can squash it into 1/3, I kept it that way because
> > even without 1/3 this variable is unused.
> 
> Hmm, the helper looks like the below. In patch 1/3 you removed
> the 'if (unlikely(!task))' test where the variable was used before,
> so 2/3 without the 1/3 would result in a compile error.

Why a compile error? It emits a warning.

> BPF_CALL_0(bpf_get_current_uid_gid)
> {
>   struct task_struct *task = current;
>   kuid_t uid;
>   kgid_t gid;
> 
>   if (unlikely(!task))
>   return -EINVAL;

Well, this is the only "user". Okay.

>   current_uid_gid(, );

Here we use current. So, task was always in vain.

So, I can happily squash 2/3 into 1/3 and resent.
The series just represented the way I've worked on the code... 

Thanks,
//richard

Re: [PATCH 2/3] bpf: Remove dead variable

[Daniel Borkmann]

On 10/16/2017 08:59 PM, Richard Weinberger wrote:

Am Montag, 16. Oktober 2017, 20:54:36 CEST schrieb Daniel Borkmann:

On 10/16/2017 08:18 PM, Richard Weinberger wrote:

task is never used in bpf_get_current_uid_gid(), kill it.

Signed-off-by: Richard Weinberger 
---

   kernel/bpf/helpers.c | 1 -
   1 file changed, 1 deletion(-)

diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index e8845adcd15e..511c9d522cfc 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -131,7 +131,6 @@ const struct bpf_func_proto
bpf_get_current_pid_tgid_proto = {>
   BPF_CALL_0(bpf_get_current_uid_gid)
   {

-   struct task_struct *task = current;

kuid_t uid;
kgid_t gid;


Needs to be squashed into patch 1/3?


I can squash it into 1/3, I kept it that way because
even without 1/3 this variable is unused.


Hmm, the helper looks like the below. In patch 1/3 you removed
the 'if (unlikely(!task))' test where the variable was used before,
so 2/3 without the 1/3 would result in a compile error.

BPF_CALL_0(bpf_get_current_uid_gid)
{
struct task_struct *task = current;
kuid_t uid;
kgid_t gid;

if (unlikely(!task))
return -EINVAL;

current_uid_gid(, );
return (u64) from_kgid(_user_ns, gid) << 32 |
 from_kuid(_user_ns, uid);
}

Re: [PATCH 2/3] bpf: Remove dead variable

[Daniel Borkmann]

On 10/16/2017 08:59 PM, Richard Weinberger wrote:

Am Montag, 16. Oktober 2017, 20:54:36 CEST schrieb Daniel Borkmann:

On 10/16/2017 08:18 PM, Richard Weinberger wrote:

task is never used in bpf_get_current_uid_gid(), kill it.

Signed-off-by: Richard Weinberger 
---

   kernel/bpf/helpers.c | 1 -
   1 file changed, 1 deletion(-)

diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index e8845adcd15e..511c9d522cfc 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -131,7 +131,6 @@ const struct bpf_func_proto
bpf_get_current_pid_tgid_proto = {>
   BPF_CALL_0(bpf_get_current_uid_gid)
   {

-   struct task_struct *task = current;

kuid_t uid;
kgid_t gid;


Needs to be squashed into patch 1/3?


I can squash it into 1/3, I kept it that way because
even without 1/3 this variable is unused.


Hmm, the helper looks like the below. In patch 1/3 you removed
the 'if (unlikely(!task))' test where the variable was used before,
so 2/3 without the 1/3 would result in a compile error.

BPF_CALL_0(bpf_get_current_uid_gid)
{
struct task_struct *task = current;
kuid_t uid;
kgid_t gid;

if (unlikely(!task))
return -EINVAL;

current_uid_gid(, );
return (u64) from_kgid(_user_ns, gid) << 32 |
 from_kuid(_user_ns, uid);
}

Re: [PATCH 2/3] bpf: Remove dead variable

[Richard Weinberger]

Am Montag, 16. Oktober 2017, 20:54:36 CEST schrieb Daniel Borkmann:
> On 10/16/2017 08:18 PM, Richard Weinberger wrote:
> > task is never used in bpf_get_current_uid_gid(), kill it.
> > 
> > Signed-off-by: Richard Weinberger 
> > ---
> > 
> >   kernel/bpf/helpers.c | 1 -
> >   1 file changed, 1 deletion(-)
> > 
> > diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> > index e8845adcd15e..511c9d522cfc 100644
> > --- a/kernel/bpf/helpers.c
> > +++ b/kernel/bpf/helpers.c
> > @@ -131,7 +131,6 @@ const struct bpf_func_proto
> > bpf_get_current_pid_tgid_proto = {> 
> >   BPF_CALL_0(bpf_get_current_uid_gid)
> >   {
> > 
> > -   struct task_struct *task = current;
> > 
> > kuid_t uid;
> > kgid_t gid;
> 
> Needs to be squashed into patch 1/3?

I can squash it into 1/3, I kept it that way because
even without 1/3 this variable is unused.

Thanks,
//richard

Re: [PATCH 2/3] bpf: Remove dead variable

[Richard Weinberger]

Am Montag, 16. Oktober 2017, 20:54:36 CEST schrieb Daniel Borkmann:
> On 10/16/2017 08:18 PM, Richard Weinberger wrote:
> > task is never used in bpf_get_current_uid_gid(), kill it.
> > 
> > Signed-off-by: Richard Weinberger 
> > ---
> > 
> >   kernel/bpf/helpers.c | 1 -
> >   1 file changed, 1 deletion(-)
> > 
> > diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> > index e8845adcd15e..511c9d522cfc 100644
> > --- a/kernel/bpf/helpers.c
> > +++ b/kernel/bpf/helpers.c
> > @@ -131,7 +131,6 @@ const struct bpf_func_proto
> > bpf_get_current_pid_tgid_proto = {> 
> >   BPF_CALL_0(bpf_get_current_uid_gid)
> >   {
> > 
> > -   struct task_struct *task = current;
> > 
> > kuid_t uid;
> > kgid_t gid;
> 
> Needs to be squashed into patch 1/3?

I can squash it into 1/3, I kept it that way because
even without 1/3 this variable is unused.

Thanks,
//richard

Re: [PATCH 2/3] bpf: Remove dead variable

[Daniel Borkmann]

On 10/16/2017 08:18 PM, Richard Weinberger wrote:

task is never used in bpf_get_current_uid_gid(), kill it.

Signed-off-by: Richard Weinberger 
---
  kernel/bpf/helpers.c | 1 -
  1 file changed, 1 deletion(-)

diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index e8845adcd15e..511c9d522cfc 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -131,7 +131,6 @@ const struct bpf_func_proto bpf_get_current_pid_tgid_proto 
= {

  BPF_CALL_0(bpf_get_current_uid_gid)
  {
-   struct task_struct *task = current;
kuid_t uid;
kgid_t gid;




Needs to be squashed into patch 1/3?

Re: [PATCH 2/3] bpf: Remove dead variable

[Daniel Borkmann]

On 10/16/2017 08:18 PM, Richard Weinberger wrote:

task is never used in bpf_get_current_uid_gid(), kill it.

Signed-off-by: Richard Weinberger 
---
  kernel/bpf/helpers.c | 1 -
  1 file changed, 1 deletion(-)

diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index e8845adcd15e..511c9d522cfc 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -131,7 +131,6 @@ const struct bpf_func_proto bpf_get_current_pid_tgid_proto 
= {

  BPF_CALL_0(bpf_get_current_uid_gid)
  {
-   struct task_struct *task = current;
kuid_t uid;
kgid_t gid;




Needs to be squashed into patch 1/3?