Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1297650 > unrolled thread
| Started by | Ross Zwisler <ross.zwisler@linux.intel.com> |
|---|---|
| First post | 2015-12-23 22:40 +0100 |
| Last post | 2015-12-23 22:50 +0100 |
| Articles | 2 — 2 participants |
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.
Re: [PATCH] cgroup: Fix uninitialized variable warning Ross Zwisler <ross.zwisler@linux.intel.com> - 2015-12-23 22:40 +0100
Re: [PATCH] cgroup: Fix uninitialized variable warning Tejun Heo <tj@kernel.org> - 2015-12-23 22:50 +0100
| From | Ross Zwisler <ross.zwisler@linux.intel.com> |
|---|---|
| Date | 2015-12-23 22:40 +0100 |
| Subject | Re: [PATCH] cgroup: Fix uninitialized variable warning |
| Message-ID | <qIZiy-6pz-13@gated-at.bofh.it> |
On Wed, Dec 23, 2015 at 04:35:19PM -0500, Tejun Heo wrote:
> Hello, Ross.
>
> On Wed, Dec 23, 2015 at 02:30:40PM -0700, Ross Zwisler wrote:
> > static int mem_cgroup_can_attach(struct cgroup_taskset *tset)
> > {
> > struct cgroup_subsys_state *css;
> > - struct mem_cgroup *memcg;
> > + struct mem_cgroup *memcg = NULL;
>
> It's one thing to add spurious init to shut up gcc
>
> > @@ -4805,7 +4805,7 @@ static int mem_cgroup_can_attach(struct cgroup_taskset *tset)
> > p = leader;
> > memcg = mem_cgroup_from_css(css);
> > }
> > - if (!p)
> > + if (!p || !memcg)
>
> and to another to add an additional processing on it.
Do you believe that the additional processing is incorrect? If somehow we
*do* get through the above loop without setting memcg, the next deref will
OOPs the kernel...
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [next] | [standalone]
| From | Tejun Heo <tj@kernel.org> |
|---|---|
| Date | 2015-12-23 22:50 +0100 |
| Message-ID | <qIZsd-6sP-5@gated-at.bofh.it> |
| In reply to | #1297650 |
On Wed, Dec 23, 2015 at 02:38:13PM -0700, Ross Zwisler wrote:
> On Wed, Dec 23, 2015 at 04:35:19PM -0500, Tejun Heo wrote:
> > Hello, Ross.
> >
> > On Wed, Dec 23, 2015 at 02:30:40PM -0700, Ross Zwisler wrote:
> > > static int mem_cgroup_can_attach(struct cgroup_taskset *tset)
> > > {
> > > struct cgroup_subsys_state *css;
> > > - struct mem_cgroup *memcg;
> > > + struct mem_cgroup *memcg = NULL;
> >
> > It's one thing to add spurious init to shut up gcc
> >
> > > @@ -4805,7 +4805,7 @@ static int mem_cgroup_can_attach(struct cgroup_taskset *tset)
> > > p = leader;
> > > memcg = mem_cgroup_from_css(css);
> > > }
> > > - if (!p)
> > > + if (!p || !memcg)
> >
> > and to another to add an additional processing on it.
>
> Do you believe that the additional processing is incorrect? If somehow we
> *do* get through the above loop without setting memcg, the next deref will
> OOPs the kernel...
That'd be a a plain kernel bug and oopsing is fine. If such
conditions are particular (more likely, more difficult to debut,
whatever), we sometimes add WARNs for them but we don't generally go
around and add spurious checks. It actually is deterimental to
readibility as people reading the code constantly have to go "when can
p && !memcg can happen? why is this explicitly checked?".
Thanks.
--
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web