Re: [2.6 patch] kernel/cgroup.c: remove dead code
pj wrote: > Check out the assembly code generated by: > > BUG_ON(sizeof(cgrp->root->release_agent_path) < PATH_MAX)); > > (Hint: you can't find it ;) > > It -is- compile time! To be clear, BUG_ON() in general is a runtime check. But the compiler can optimize out constant expressions, and code conditionally executed in the case of a constant that can never be true. Adrian wrote: > > It -is- compile time! > > But when the condition is fulfilled, you get a runtime error, not a > compile error. Correct you are. And when I advocated BUG_ON in this role, I did two things: 1) I was blissfully ignorant of BUILD_BUG_ON(), which would be better here, for the reasons you state, and 2) I did a silent calculation in my head, noticing that if the constants ever changed so as to trigger this check, it would show up really quickly in testing, because it was on code path that would be hard to miss. Because of this, the delay until runtime of this check was less of a disaster than it would have been on a rarely traveled code path. In sum - Adrian is right - use BUILD_BUG_ON() here. Thanks, Adrian. -- I won't rest till it's the best ... Programmer, Linux Scalability Paul Jackson <[EMAIL PROTECTED]> 1.925.600.0401 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [2.6 patch] kernel/cgroup.c: remove dead code
On Thu, Oct 25, 2007 at 06:24:25PM -0700, Paul Jackson wrote: > Paul M wrote: > > Sounds reasonable to me. Is there any kind of compile-time assert > > macro in the kernel? > > Check out the assembly code generated by: > > BUG_ON(sizeof(cgrp->root->release_agent_path) < PATH_MAX)); > > (Hint: you can't find it ;) > > It -is- compile time! But when the condition is fulfilled, you get a runtime error, not a compile error. cu Adrian -- "Is there not promise of rain?" Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. "Only a promise," Lao Er said. Pearl S. Buck - Dragon Seed - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [2.6 patch] kernel/cgroup.c: remove dead code
On 10/25/07, Paul Jackson <[EMAIL PROTECTED]> wrote: > Paul M wrote: > > Sounds reasonable to me. Is there any kind of compile-time assert > > macro in the kernel? > > Check out the assembly code generated by: > > BUG_ON(sizeof(cgrp->root->release_agent_path) < PATH_MAX)); > > (Hint: you can't find it ;) > > It -is- compile time! > Sounds like a good solution. Paul - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [2.6 patch] kernel/cgroup.c: remove dead code
Paul M wrote: > Sounds reasonable to me. Is there any kind of compile-time assert > macro in the kernel? Check out the assembly code generated by: BUG_ON(sizeof(cgrp->root->release_agent_path) < PATH_MAX)); (Hint: you can't find it ;) It -is- compile time! -- I won't rest till it's the best ... Programmer, Linux Scalability Paul Jackson <[EMAIL PROTECTED]> 1.925.600.0401 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [2.6 patch] kernel/cgroup.c: remove dead code
On Thu, Oct 25, 2007 at 06:10:24PM -0700, Paul Menage wrote: > On 10/24/07, Paul Jackson <[EMAIL PROTECTED]> wrote: > > Paul M wrote: > > > I think I'd rather not make this change - if we later changed the size > > > of release_agent_path[] this could silently fail. Can we get around > > > the coverity checker somehow? > > > > Perhaps we can simplify this check then, to: > > > > BUG_ON(sizeof(cgrp->root->release_agent_path) < PATH_MAX)); > > > > Less runtime code. > > Sounds reasonable to me. Is there any kind of compile-time assert > macro in the kernel? BUILD_BUG_ON() > Paul cu Adrian -- "Is there not promise of rain?" Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. "Only a promise," Lao Er said. Pearl S. Buck - Dragon Seed - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [2.6 patch] kernel/cgroup.c: remove dead code
On 10/24/07, Paul Jackson <[EMAIL PROTECTED]> wrote: > Paul M wrote: > > I think I'd rather not make this change - if we later changed the size > > of release_agent_path[] this could silently fail. Can we get around > > the coverity checker somehow? > > Perhaps we can simplify this check then, to: > > BUG_ON(sizeof(cgrp->root->release_agent_path) < PATH_MAX)); > > Less runtime code. Sounds reasonable to me. Is there any kind of compile-time assert macro in the kernel? Paul - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [2.6 patch] kernel/cgroup.c: remove dead code
On 10/24/07, Paul Jackson [EMAIL PROTECTED] wrote: Paul M wrote: I think I'd rather not make this change - if we later changed the size of release_agent_path[] this could silently fail. Can we get around the coverity checker somehow? Perhaps we can simplify this check then, to: BUG_ON(sizeof(cgrp-root-release_agent_path) PATH_MAX)); Less runtime code. Sounds reasonable to me. Is there any kind of compile-time assert macro in the kernel? Paul - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [2.6 patch] kernel/cgroup.c: remove dead code
On Thu, Oct 25, 2007 at 06:10:24PM -0700, Paul Menage wrote: On 10/24/07, Paul Jackson [EMAIL PROTECTED] wrote: Paul M wrote: I think I'd rather not make this change - if we later changed the size of release_agent_path[] this could silently fail. Can we get around the coverity checker somehow? Perhaps we can simplify this check then, to: BUG_ON(sizeof(cgrp-root-release_agent_path) PATH_MAX)); Less runtime code. Sounds reasonable to me. Is there any kind of compile-time assert macro in the kernel? BUILD_BUG_ON() Paul cu Adrian -- Is there not promise of rain? Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. Only a promise, Lao Er said. Pearl S. Buck - Dragon Seed - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [2.6 patch] kernel/cgroup.c: remove dead code
Paul M wrote: Sounds reasonable to me. Is there any kind of compile-time assert macro in the kernel? Check out the assembly code generated by: BUG_ON(sizeof(cgrp-root-release_agent_path) PATH_MAX)); (Hint: you can't find it ;) It -is- compile time! -- I won't rest till it's the best ... Programmer, Linux Scalability Paul Jackson [EMAIL PROTECTED] 1.925.600.0401 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [2.6 patch] kernel/cgroup.c: remove dead code
On 10/25/07, Paul Jackson [EMAIL PROTECTED] wrote: Paul M wrote: Sounds reasonable to me. Is there any kind of compile-time assert macro in the kernel? Check out the assembly code generated by: BUG_ON(sizeof(cgrp-root-release_agent_path) PATH_MAX)); (Hint: you can't find it ;) It -is- compile time! Sounds like a good solution. Paul - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [2.6 patch] kernel/cgroup.c: remove dead code
On Thu, Oct 25, 2007 at 06:24:25PM -0700, Paul Jackson wrote: Paul M wrote: Sounds reasonable to me. Is there any kind of compile-time assert macro in the kernel? Check out the assembly code generated by: BUG_ON(sizeof(cgrp-root-release_agent_path) PATH_MAX)); (Hint: you can't find it ;) It -is- compile time! But when the condition is fulfilled, you get a runtime error, not a compile error. cu Adrian -- Is there not promise of rain? Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. Only a promise, Lao Er said. Pearl S. Buck - Dragon Seed - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [2.6 patch] kernel/cgroup.c: remove dead code
pj wrote: Check out the assembly code generated by: BUG_ON(sizeof(cgrp-root-release_agent_path) PATH_MAX)); (Hint: you can't find it ;) It -is- compile time! To be clear, BUG_ON() in general is a runtime check. But the compiler can optimize out constant expressions, and code conditionally executed in the case of a constant that can never be true. Adrian wrote: It -is- compile time! But when the condition is fulfilled, you get a runtime error, not a compile error. Correct you are. And when I advocated BUG_ON in this role, I did two things: 1) I was blissfully ignorant of BUILD_BUG_ON(), which would be better here, for the reasons you state, and 2) I did a silent calculation in my head, noticing that if the constants ever changed so as to trigger this check, it would show up really quickly in testing, because it was on code path that would be hard to miss. Because of this, the delay until runtime of this check was less of a disaster than it would have been on a rarely traveled code path. In sum - Adrian is right - use BUILD_BUG_ON() here. Thanks, Adrian. -- I won't rest till it's the best ... Programmer, Linux Scalability Paul Jackson [EMAIL PROTECTED] 1.925.600.0401 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [2.6 patch] kernel/cgroup.c: remove dead code
Paul M wrote: > I think I'd rather not make this change - if we later changed the size > of release_agent_path[] this could silently fail. Can we get around > the coverity checker somehow? Perhaps we can simplify this check then, to: BUG_ON(sizeof(cgrp->root->release_agent_path) < PATH_MAX)); Less runtime code. This patch of Adrian highlighted a couple more opportunities for code tweaking ... see my RFC patches, coming next from me. -- I won't rest till it's the best ... Programmer, Linux Scalability Paul Jackson <[EMAIL PROTECTED]> 1.925.600.0401 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [2.6 patch] kernel/cgroup.c: remove dead code
On 10/24/07, Adrian Bunk <[EMAIL PROTECTED]> wrote: > > Two questions: > - Is it really intended to perhaps change release_agent_path[] to have > less than PATH_MAX size? I've got no intention to do so currently. > - If yes, do you want to return -E2BIG for (nbytes >= PATH_MAX) or for > (nbytes >= sizeof(root->release_agent_path)) ? I think E2BIG for the former for backwards compatabilty; the latter could be either ENOSPC or E2BIG; i.e. both checks are useful - one to stop us allocating more memory than is sensible, and one to stop us overrunning the buffer; the fact that these two are the same size at the moment is coincidence. I guess ideally the first check would be for the max() of any of the data structures that we expect to be able to write over; PATH_MAX was just picked as a convenience. Paul - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [2.6 patch] kernel/cgroup.c: remove dead code
On Wed, Oct 24, 2007 at 09:30:23AM -0700, Paul Menage wrote:
> I think I'd rather not make this change - if we later changed the size
> of release_agent_path[] this could silently fail. Can we get around
> the coverity checker somehow?
I do not care what the Coverity checker thinks about the code, and
there's no reason for changing code only for the sake of this checker.
Two questions:
- Is it really intended to perhaps change release_agent_path[] to have
less than PATH_MAX size?
- If yes, do you want to return -E2BIG for (nbytes >= PATH_MAX) or for
(nbytes >= sizeof(root->release_agent_path)) ?
> Paul
>
> On 10/24/07, Adrian Bunk <[EMAIL PROTECTED]> wrote:
> > This patch removes dead code spotted by the Coverity checker
> > (look at the "(nbytes >= PATH_MAX)" check).
> >
> > Signed-off-by: Adrian Bunk <[EMAIL PROTECTED]>
> >
> > ---
> >
> > kernel/cgroup.c | 18 --
> > 1 file changed, 8 insertions(+), 10 deletions(-)
> >
> > --- linux-2.6/kernel/cgroup.c.old 2007-10-23 18:37:43.0 +0200
> > +++ linux-2.6/kernel/cgroup.c 2007-10-23 18:39:15.0 +0200
> > @@ -1320,90 +1320,88 @@ static ssize_t cgroup_common_file_write(
> >
> > if (nbytes >= PATH_MAX)
> > return -E2BIG;
> >
> > /* +1 for nul-terminator */
> > buffer = kmalloc(nbytes + 1, GFP_KERNEL);
> > if (buffer == NULL)
> > return -ENOMEM;
> >
> > if (copy_from_user(buffer, userbuf, nbytes)) {
> > retval = -EFAULT;
> > goto out1;
> > }
> > buffer[nbytes] = 0; /* nul-terminate */
> >
> > mutex_lock(_mutex);
> >
> > if (cgroup_is_removed(cgrp)) {
> > retval = -ENODEV;
> > goto out2;
> > }
> >
> > switch (type) {
> > case FILE_TASKLIST:
> > retval = attach_task_by_pid(cgrp, buffer);
> > break;
> > case FILE_NOTIFY_ON_RELEASE:
> > clear_bit(CGRP_RELEASABLE, >flags);
> > if (simple_strtoul(buffer, NULL, 10) != 0)
> > set_bit(CGRP_NOTIFY_ON_RELEASE, >flags);
> > else
> > clear_bit(CGRP_NOTIFY_ON_RELEASE, >flags);
> > break;
> > case FILE_RELEASE_AGENT:
> > {
> > struct cgroupfs_root *root = cgrp->root;
> > /* Strip trailing newline */
> > if (nbytes && (buffer[nbytes-1] == '\n')) {
> > buffer[nbytes-1] = 0;
> > }
> > - if (nbytes < sizeof(root->release_agent_path)) {
> > - /* We never write anything other than '\0'
> > -* into the last char of release_agent_path,
> > -* so it always remains a NUL-terminated
> > -* string */
> > - strncpy(root->release_agent_path, buffer, nbytes);
> > - root->release_agent_path[nbytes] = 0;
> > - } else {
> > - retval = -ENOSPC;
> > - }
> > +
> > + /* We never write anything other than '\0'
> > +* into the last char of release_agent_path,
> > +* so it always remains a NUL-terminated
> > +* string */
> > + strncpy(root->release_agent_path, buffer, nbytes);
> > + root->release_agent_path[nbytes] = 0;
> > +
> > break;
> > }
> > default:
> > retval = -EINVAL;
> > goto out2;
> > }
> >
> > if (retval == 0)
> > retval = nbytes;
> > out2:
> > mutex_unlock(_mutex);
> > out1:
> > kfree(buffer);
> > return retval;
> > }
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [2.6 patch] kernel/cgroup.c: remove dead code
I think I'd rather not make this change - if we later changed the size
of release_agent_path[] this could silently fail. Can we get around
the coverity checker somehow?
Paul
On 10/24/07, Adrian Bunk <[EMAIL PROTECTED]> wrote:
> This patch removes dead code spotted by the Coverity checker
> (look at the "(nbytes >= PATH_MAX)" check).
>
> Signed-off-by: Adrian Bunk <[EMAIL PROTECTED]>
>
> ---
>
> kernel/cgroup.c | 18 --
> 1 file changed, 8 insertions(+), 10 deletions(-)
>
> --- linux-2.6/kernel/cgroup.c.old 2007-10-23 18:37:43.0 +0200
> +++ linux-2.6/kernel/cgroup.c 2007-10-23 18:39:15.0 +0200
> @@ -1320,90 +1320,88 @@ static ssize_t cgroup_common_file_write(
>
> if (nbytes >= PATH_MAX)
> return -E2BIG;
>
> /* +1 for nul-terminator */
> buffer = kmalloc(nbytes + 1, GFP_KERNEL);
> if (buffer == NULL)
> return -ENOMEM;
>
> if (copy_from_user(buffer, userbuf, nbytes)) {
> retval = -EFAULT;
> goto out1;
> }
> buffer[nbytes] = 0; /* nul-terminate */
>
> mutex_lock(_mutex);
>
> if (cgroup_is_removed(cgrp)) {
> retval = -ENODEV;
> goto out2;
> }
>
> switch (type) {
> case FILE_TASKLIST:
> retval = attach_task_by_pid(cgrp, buffer);
> break;
> case FILE_NOTIFY_ON_RELEASE:
> clear_bit(CGRP_RELEASABLE, >flags);
> if (simple_strtoul(buffer, NULL, 10) != 0)
> set_bit(CGRP_NOTIFY_ON_RELEASE, >flags);
> else
> clear_bit(CGRP_NOTIFY_ON_RELEASE, >flags);
> break;
> case FILE_RELEASE_AGENT:
> {
> struct cgroupfs_root *root = cgrp->root;
> /* Strip trailing newline */
> if (nbytes && (buffer[nbytes-1] == '\n')) {
> buffer[nbytes-1] = 0;
> }
> - if (nbytes < sizeof(root->release_agent_path)) {
> - /* We never write anything other than '\0'
> -* into the last char of release_agent_path,
> -* so it always remains a NUL-terminated
> -* string */
> - strncpy(root->release_agent_path, buffer, nbytes);
> - root->release_agent_path[nbytes] = 0;
> - } else {
> - retval = -ENOSPC;
> - }
> +
> + /* We never write anything other than '\0'
> +* into the last char of release_agent_path,
> +* so it always remains a NUL-terminated
> +* string */
> + strncpy(root->release_agent_path, buffer, nbytes);
> + root->release_agent_path[nbytes] = 0;
> +
> break;
> }
> default:
> retval = -EINVAL;
> goto out2;
> }
>
> if (retval == 0)
> retval = nbytes;
> out2:
> mutex_unlock(_mutex);
> out1:
> kfree(buffer);
> return retval;
> }
>
> static ssize_t cgroup_file_write(struct file *file, const char __user *buf,
> size_t nbytes, loff_t *ppos)
> {
> struct cftype *cft = __d_cft(file->f_dentry);
> struct cgroup *cgrp = __d_cgrp(file->f_dentry->d_parent);
>
> if (!cft)
> return -ENODEV;
> if (cft->write)
> return cft->write(cgrp, cft, file, buf, nbytes, ppos);
> if (cft->write_uint)
> return cgroup_write_uint(cgrp, cft, file, buf, nbytes, ppos);
> return -EINVAL;
> }
>
> static ssize_t cgroup_read_uint(struct cgroup *cgrp, struct cftype *cft,
>struct file *file,
>char __user *buf, size_t nbytes,
>loff_t *ppos)
> {
> char tmp[64];
> u64 val = cft->read_uint(cgrp, cft);
> int len = sprintf(tmp, "%llu\n", (unsigned long long) val);
>
>
>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [2.6 patch] kernel/cgroup.c: remove dead code
I think I'd rather not make this change - if we later changed the size
of release_agent_path[] this could silently fail. Can we get around
the coverity checker somehow?
Paul
On 10/24/07, Adrian Bunk [EMAIL PROTECTED] wrote:
This patch removes dead code spotted by the Coverity checker
(look at the (nbytes = PATH_MAX) check).
Signed-off-by: Adrian Bunk [EMAIL PROTECTED]
---
kernel/cgroup.c | 18 --
1 file changed, 8 insertions(+), 10 deletions(-)
--- linux-2.6/kernel/cgroup.c.old 2007-10-23 18:37:43.0 +0200
+++ linux-2.6/kernel/cgroup.c 2007-10-23 18:39:15.0 +0200
@@ -1320,90 +1320,88 @@ static ssize_t cgroup_common_file_write(
if (nbytes = PATH_MAX)
return -E2BIG;
/* +1 for nul-terminator */
buffer = kmalloc(nbytes + 1, GFP_KERNEL);
if (buffer == NULL)
return -ENOMEM;
if (copy_from_user(buffer, userbuf, nbytes)) {
retval = -EFAULT;
goto out1;
}
buffer[nbytes] = 0; /* nul-terminate */
mutex_lock(cgroup_mutex);
if (cgroup_is_removed(cgrp)) {
retval = -ENODEV;
goto out2;
}
switch (type) {
case FILE_TASKLIST:
retval = attach_task_by_pid(cgrp, buffer);
break;
case FILE_NOTIFY_ON_RELEASE:
clear_bit(CGRP_RELEASABLE, cgrp-flags);
if (simple_strtoul(buffer, NULL, 10) != 0)
set_bit(CGRP_NOTIFY_ON_RELEASE, cgrp-flags);
else
clear_bit(CGRP_NOTIFY_ON_RELEASE, cgrp-flags);
break;
case FILE_RELEASE_AGENT:
{
struct cgroupfs_root *root = cgrp-root;
/* Strip trailing newline */
if (nbytes (buffer[nbytes-1] == '\n')) {
buffer[nbytes-1] = 0;
}
- if (nbytes sizeof(root-release_agent_path)) {
- /* We never write anything other than '\0'
-* into the last char of release_agent_path,
-* so it always remains a NUL-terminated
-* string */
- strncpy(root-release_agent_path, buffer, nbytes);
- root-release_agent_path[nbytes] = 0;
- } else {
- retval = -ENOSPC;
- }
+
+ /* We never write anything other than '\0'
+* into the last char of release_agent_path,
+* so it always remains a NUL-terminated
+* string */
+ strncpy(root-release_agent_path, buffer, nbytes);
+ root-release_agent_path[nbytes] = 0;
+
break;
}
default:
retval = -EINVAL;
goto out2;
}
if (retval == 0)
retval = nbytes;
out2:
mutex_unlock(cgroup_mutex);
out1:
kfree(buffer);
return retval;
}
static ssize_t cgroup_file_write(struct file *file, const char __user *buf,
size_t nbytes, loff_t *ppos)
{
struct cftype *cft = __d_cft(file-f_dentry);
struct cgroup *cgrp = __d_cgrp(file-f_dentry-d_parent);
if (!cft)
return -ENODEV;
if (cft-write)
return cft-write(cgrp, cft, file, buf, nbytes, ppos);
if (cft-write_uint)
return cgroup_write_uint(cgrp, cft, file, buf, nbytes, ppos);
return -EINVAL;
}
static ssize_t cgroup_read_uint(struct cgroup *cgrp, struct cftype *cft,
struct file *file,
char __user *buf, size_t nbytes,
loff_t *ppos)
{
char tmp[64];
u64 val = cft-read_uint(cgrp, cft);
int len = sprintf(tmp, %llu\n, (unsigned long long) val);
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [2.6 patch] kernel/cgroup.c: remove dead code
On Wed, Oct 24, 2007 at 09:30:23AM -0700, Paul Menage wrote:
I think I'd rather not make this change - if we later changed the size
of release_agent_path[] this could silently fail. Can we get around
the coverity checker somehow?
I do not care what the Coverity checker thinks about the code, and
there's no reason for changing code only for the sake of this checker.
Two questions:
- Is it really intended to perhaps change release_agent_path[] to have
less than PATH_MAX size?
- If yes, do you want to return -E2BIG for (nbytes = PATH_MAX) or for
(nbytes = sizeof(root-release_agent_path)) ?
Paul
On 10/24/07, Adrian Bunk [EMAIL PROTECTED] wrote:
This patch removes dead code spotted by the Coverity checker
(look at the (nbytes = PATH_MAX) check).
Signed-off-by: Adrian Bunk [EMAIL PROTECTED]
---
kernel/cgroup.c | 18 --
1 file changed, 8 insertions(+), 10 deletions(-)
--- linux-2.6/kernel/cgroup.c.old 2007-10-23 18:37:43.0 +0200
+++ linux-2.6/kernel/cgroup.c 2007-10-23 18:39:15.0 +0200
@@ -1320,90 +1320,88 @@ static ssize_t cgroup_common_file_write(
if (nbytes = PATH_MAX)
return -E2BIG;
/* +1 for nul-terminator */
buffer = kmalloc(nbytes + 1, GFP_KERNEL);
if (buffer == NULL)
return -ENOMEM;
if (copy_from_user(buffer, userbuf, nbytes)) {
retval = -EFAULT;
goto out1;
}
buffer[nbytes] = 0; /* nul-terminate */
mutex_lock(cgroup_mutex);
if (cgroup_is_removed(cgrp)) {
retval = -ENODEV;
goto out2;
}
switch (type) {
case FILE_TASKLIST:
retval = attach_task_by_pid(cgrp, buffer);
break;
case FILE_NOTIFY_ON_RELEASE:
clear_bit(CGRP_RELEASABLE, cgrp-flags);
if (simple_strtoul(buffer, NULL, 10) != 0)
set_bit(CGRP_NOTIFY_ON_RELEASE, cgrp-flags);
else
clear_bit(CGRP_NOTIFY_ON_RELEASE, cgrp-flags);
break;
case FILE_RELEASE_AGENT:
{
struct cgroupfs_root *root = cgrp-root;
/* Strip trailing newline */
if (nbytes (buffer[nbytes-1] == '\n')) {
buffer[nbytes-1] = 0;
}
- if (nbytes sizeof(root-release_agent_path)) {
- /* We never write anything other than '\0'
-* into the last char of release_agent_path,
-* so it always remains a NUL-terminated
-* string */
- strncpy(root-release_agent_path, buffer, nbytes);
- root-release_agent_path[nbytes] = 0;
- } else {
- retval = -ENOSPC;
- }
+
+ /* We never write anything other than '\0'
+* into the last char of release_agent_path,
+* so it always remains a NUL-terminated
+* string */
+ strncpy(root-release_agent_path, buffer, nbytes);
+ root-release_agent_path[nbytes] = 0;
+
break;
}
default:
retval = -EINVAL;
goto out2;
}
if (retval == 0)
retval = nbytes;
out2:
mutex_unlock(cgroup_mutex);
out1:
kfree(buffer);
return retval;
}
cu
Adrian
--
Is there not promise of rain? Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
Only a promise, Lao Er said.
Pearl S. Buck - Dragon Seed
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [2.6 patch] kernel/cgroup.c: remove dead code
On 10/24/07, Adrian Bunk [EMAIL PROTECTED] wrote: Two questions: - Is it really intended to perhaps change release_agent_path[] to have less than PATH_MAX size? I've got no intention to do so currently. - If yes, do you want to return -E2BIG for (nbytes = PATH_MAX) or for (nbytes = sizeof(root-release_agent_path)) ? I think E2BIG for the former for backwards compatabilty; the latter could be either ENOSPC or E2BIG; i.e. both checks are useful - one to stop us allocating more memory than is sensible, and one to stop us overrunning the buffer; the fact that these two are the same size at the moment is coincidence. I guess ideally the first check would be for the max() of any of the data structures that we expect to be able to write over; PATH_MAX was just picked as a convenience. Paul - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [2.6 patch] kernel/cgroup.c: remove dead code
Paul M wrote: I think I'd rather not make this change - if we later changed the size of release_agent_path[] this could silently fail. Can we get around the coverity checker somehow? Perhaps we can simplify this check then, to: BUG_ON(sizeof(cgrp-root-release_agent_path) PATH_MAX)); Less runtime code. This patch of Adrian highlighted a couple more opportunities for code tweaking ... see my RFC patches, coming next from me. -- I won't rest till it's the best ... Programmer, Linux Scalability Paul Jackson [EMAIL PROTECTED] 1.925.600.0401 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
