Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1607944 > unrolled thread

Re: [PATCH 09/46] selinux: Delete an error message for a failed memory allocation in policydb_read()

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 09/46] selinux: Delete an error message for a failed  memory allocation in policydb_read() Paul Moore <paul@paul-moore.com> - 2017-03-23 22:40 +0100
    Re: selinux: Delete an error message for a failed memory allocation  in policydb_read() SF Markus Elfring <elfring@users.sourceforge.net> - 2017-03-24 13:20 +0100
      Re: selinux: Delete an error message for a failed memory allocation  in policydb_read() Paul Moore <paul@paul-moore.com> - 2017-03-25 16:50 +0100
        Re: selinux: Delete an error message for a failed memory allocation  in policydb_read() SF Markus Elfring <elfring@users.sourceforge.net> - 2017-03-27 08:10 +0200
          Re: selinux: Delete an error message for a failed memory allocation  in policydb_read() Paul Moore <paul@paul-moore.com> - 2017-03-27 20:40 +0200

#1607944 — Re: [PATCH 09/46] selinux: Delete an error message for a failed memory allocation in policydb_read()

On Sun, Jan 15, 2017 at 10:07 AM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sat, 14 Jan 2017 14:20:41 +0100
>
> Omit an extra message for a memory allocation failure in this function.
>
> Link: http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  security/selinux/ss/policydb.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)

I'm not going to remove an error message without some better reasoning
in the patch description.  Providing a link to slides is fine, but
your commit message needs to convey the important information and I
don't think that is the case here (what happens when that URL dies?).

> diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
> index fe8992382a71..53e6d06e772a 100644
> --- a/security/selinux/ss/policydb.c
> +++ b/security/selinux/ss/policydb.c
> @@ -2269,11 +2269,8 @@ int policydb_read(struct policydb *p, void *fp)
>
>         rc = -ENOMEM;
>         policydb_str = kmalloc(len + 1, GFP_KERNEL);
> -       if (!policydb_str) {
> -               printk(KERN_ERR "SELinux:  unable to allocate memory for policydb "
> -                      "string of length %d\n", len);
> +       if (!policydb_str)
>                 goto bad;
> -       }
>
>         rc = next_entry(policydb_str, fp, len);
>         if (rc) {
> --
> 2.11.0
>



-- 
paul moore
www.paul-moore.com

[toc] | [next] | [standalone]

#1608366 — Re: selinux: Delete an error message for a failed memory allocation in policydb_read()

>> Omit an extra message for a memory allocation failure in this function.
>>
>> Link: http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
>> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
>> ---
>>  security/selinux/ss/policydb.c | 5 +----
>>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> I'm not going to remove an error message without some better reasoning
> in the patch description.  Providing a link to slides is fine, but
> your commit message needs to convey the important information and I
> don't think that is the case here (what happens when that URL dies?).

Do you need an explicit reminder there that the function “kmalloc” provides its own
error reporting already because the flag “__GFP_NOWARN” was not passed here?

Regards,
Markus

[toc] | [prev] | [next] | [standalone]

#1609177 — Re: selinux: Delete an error message for a failed memory allocation in policydb_read()

On Fri, Mar 24, 2017 at 8:13 AM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
>>> Omit an extra message for a memory allocation failure in this function.
>>>
>>> Link: http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
>>> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
>>> ---
>>>  security/selinux/ss/policydb.c | 5 +----
>>>  1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> I'm not going to remove an error message without some better reasoning
>> in the patch description.  Providing a link to slides is fine, but
>> your commit message needs to convey the important information and I
>> don't think that is the case here (what happens when that URL dies?).
>
> Do you need an explicit reminder there that the function “kmalloc” provides its own
> error reporting already because the flag “__GFP_NOWARN” was not passed here?

That is what I said by "better reasoning in the patch description",
however, now that I'm looking at this again, I don't think I'm going
to merge this.  Yes, maybe in some cases it is a bit wasteful, but I
like the error message.

-- 
paul moore
www.paul-moore.com

[toc] | [prev] | [next] | [standalone]

#1609478 — Re: selinux: Delete an error message for a failed memory allocation in policydb_read()

> …, but I like the error message.

How do you think about to pass the flag “__GFP_NOWARN” if you like
this information more than the default error reporting of the function “kmalloc”?

Regards,
Markus

[toc] | [prev] | [next] | [standalone]

#1610061 — Re: selinux: Delete an error message for a failed memory allocation in policydb_read()

On Mon, Mar 27, 2017 at 1:56 AM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
>> …, but I like the error message.
>
> How do you think about to pass the flag “__GFP_NOWARN” if you like
> this information more than the default error reporting of the function “kmalloc”?

Possibly, although I would encourage you to just leave it as-is for
the moment.  Reviewing and merging patches carries a cost, and I would
very much prefer to allocate my time/resources on changes that have a
more significant impact.  I don't want to discourage your from
contributing to SELinux development, but I do want to strongly
encourage you to contribute more meaningful patches.

-- 
paul moore
www.paul-moore.com

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web