| Message ID | 1395766541-23979-10-git-send-email-julien.grall@linaro.org |
|---|---|
| State | Accepted, archived |
| Headers | show |
On 25/03/14 16:55, Julien Grall wrote: > do_tmem_destroy_pool is checking if pools == NULL. But, pools is a fixed > array. > > Clang 3.5 will fail to compile xen/common/tmem.c with the following error: > tmem.c:1848:18: error: comparison of array 'client->pools' equal to a null pointer is always false [-Werror,-Wtautological-pointer-compare] > if ( client->pools == NULL ) Coverity-ID:1055632 ~Andrew > Signed-off-by: Julien Grall <julien.grall@linaro.org> > Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > --- > xen/common/tmem.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/xen/common/tmem.c b/xen/common/tmem.c > index 02e7e2e..0a24b3f 100644 > --- a/xen/common/tmem.c > +++ b/xen/common/tmem.c > @@ -1845,8 +1845,6 @@ static int do_tmem_destroy_pool(uint32_t pool_id) > struct client *client = current->domain->tmem_client; > struct tmem_pool *pool; > > - if ( client->pools == NULL ) > - return 0; > if ( pool_id >= MAX_POOLS_PER_DOMAIN ) > return 0; > if ( (pool = client->pools[pool_id]) == NULL )
Hi Andrew, I just saw that a patch was already sent few months ago (http://lists.xenproject.org/archives/html/xen-devel/2013-11/msg03664.html), but never upstreamed. Konrad, I'm fine to drop this patch if you plan to send your series. Regards, On 03/25/2014 06:05 PM, Andrew Cooper wrote: > On 25/03/14 16:55, Julien Grall wrote: >> do_tmem_destroy_pool is checking if pools == NULL. But, pools is a fixed >> array. >> >> Clang 3.5 will fail to compile xen/common/tmem.c with the following error: >> tmem.c:1848:18: error: comparison of array 'client->pools' equal to a null pointer is always false [-Werror,-Wtautological-pointer-compare] >> if ( client->pools == NULL ) > > Coverity-ID:1055632 > > ~Andrew > >> Signed-off-by: Julien Grall <julien.grall@linaro.org> >> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> >> --- >> xen/common/tmem.c | 2 -- >> 1 file changed, 2 deletions(-) >> >> diff --git a/xen/common/tmem.c b/xen/common/tmem.c >> index 02e7e2e..0a24b3f 100644 >> --- a/xen/common/tmem.c >> +++ b/xen/common/tmem.c >> @@ -1845,8 +1845,6 @@ static int do_tmem_destroy_pool(uint32_t pool_id) >> struct client *client = current->domain->tmem_client; >> struct tmem_pool *pool; >> >> - if ( client->pools == NULL ) >> - return 0; >> if ( pool_id >= MAX_POOLS_PER_DOMAIN ) >> return 0; >> if ( (pool = client->pools[pool_id]) == NULL ) >
On Tue, Mar 25, 2014 at 06:18:21PM +0000, Julien Grall wrote: > Hi Andrew, > > I just saw that a patch was already sent few months ago > (http://lists.xenproject.org/archives/html/xen-devel/2013-11/msg03664.html), > but never upstreamed. > > Konrad, I'm fine to drop this patch if you plan to send your series. Nah, lets use this patch. Thanks! Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > > Regards, > > On 03/25/2014 06:05 PM, Andrew Cooper wrote: > > On 25/03/14 16:55, Julien Grall wrote: > >> do_tmem_destroy_pool is checking if pools == NULL. But, pools is a fixed > >> array. > >> > >> Clang 3.5 will fail to compile xen/common/tmem.c with the following error: > >> tmem.c:1848:18: error: comparison of array 'client->pools' equal to a null pointer is always false [-Werror,-Wtautological-pointer-compare] > >> if ( client->pools == NULL ) > > > > Coverity-ID:1055632 > > > > ~Andrew > > > >> Signed-off-by: Julien Grall <julien.grall@linaro.org> > >> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > >> --- > >> xen/common/tmem.c | 2 -- > >> 1 file changed, 2 deletions(-) > >> > >> diff --git a/xen/common/tmem.c b/xen/common/tmem.c > >> index 02e7e2e..0a24b3f 100644 > >> --- a/xen/common/tmem.c > >> +++ b/xen/common/tmem.c > >> @@ -1845,8 +1845,6 @@ static int do_tmem_destroy_pool(uint32_t pool_id) > >> struct client *client = current->domain->tmem_client; > >> struct tmem_pool *pool; > >> > >> - if ( client->pools == NULL ) > >> - return 0; > >> if ( pool_id >= MAX_POOLS_PER_DOMAIN ) > >> return 0; > >> if ( (pool = client->pools[pool_id]) == NULL ) > > > > > -- > Julien Grall > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
On 25/03/14 16:55, Julien Grall wrote: > do_tmem_destroy_pool is checking if pools == NULL. But, pools is a fixed > array. This patch (committed to staging as ac0f56a2) has turned up in my backport list somehow, even though (a) it wouldn't be my responsibility as it's a hypervisor patch and (b) I can find no trace of it in my mailbox to explain why. My backport list only records the identity of the patch and the date when I added it to the list (8th of April 2014 in this case). I normally use my email to find out why. Anyway, I thought I'd email Jan as stable maintainer to decide what to do about it. I'm removing it from my own backport list. Ian.
>>> On 22.05.14 at 18:01, <Ian.Jackson@eu.citrix.com> wrote: > On 25/03/14 16:55, Julien Grall wrote: >> do_tmem_destroy_pool is checking if pools == NULL. But, pools is a fixed >> array. > > This patch (committed to staging as ac0f56a2) has turned up in my > backport list somehow, even though (a) it wouldn't be my > responsibility as it's a hypervisor patch and (b) I can find no trace > of it in my mailbox to explain why. > > My backport list only records the identity of the patch and the date > when I added it to the list (8th of April 2014 in this case). I > normally use my email to find out why. Perhaps just a one-off in picking commit IDs from git log output? > Anyway, I thought I'd email Jan as stable maintainer to decide what to > do about it. I'm removing it from my own backport list. I see no reason to apply any tmem patches to stable trees until tmem's status changes back to supported, or unless being urged by the tmem maintainer. Jan
Jan Beulich writes ("Re: [Xen-devel] [PATCH 09/34] xen/common: tmem: Remove dumb check in do_tmem_destroy_pool [and 2 more messages]"):
> On 22.05.14 at 18:01, <Ian.Jackson@eu.citrix.com> wrote:
> > On 25/03/14 16:55, Julien Grall wrote:
> > This patch (committed to staging as ac0f56a2) has turned up in my
> > backport list somehow, even though (a) it wouldn't be my
> > responsibility as it's a hypervisor patch and (b) I can find no trace
> > of it in my mailbox to explain why.
> >
> > My backport list only records the identity of the patch and the date
> > when I added it to the list (8th of April 2014 in this case). I
> > normally use my email to find out why.
>
> Perhaps just a one-off in picking commit IDs from git log output?
I thought of that but the adjacent commits are not candidates either.
> > Anyway, I thought I'd email Jan as stable maintainer to decide what to
> > do about it. I'm removing it from my own backport list.
>
> I see no reason to apply any tmem patches to stable trees until
> tmem's status changes back to supported, or unless being urged
> by the tmem maintainer.
Right.
Thanks and sorry for the noise.
Ian.
>>> On 23.05.14 at 12:23, <Ian.Jackson@eu.citrix.com> wrote: > Jan Beulich writes ("Re: [Xen-devel] [PATCH 09/34] xen/common: tmem: Remove > dumb check in do_tmem_destroy_pool [and 2 more messages]"): >> On 22.05.14 at 18:01, <Ian.Jackson@eu.citrix.com> wrote: >> > On 25/03/14 16:55, Julien Grall wrote: >> > This patch (committed to staging as ac0f56a2) has turned up in my >> > backport list somehow, even though (a) it wouldn't be my >> > responsibility as it's a hypervisor patch and (b) I can find no trace >> > of it in my mailbox to explain why. >> > >> > My backport list only records the identity of the patch and the date >> > when I added it to the list (8th of April 2014 in this case). I >> > normally use my email to find out why. >> >> Perhaps just a one-off in picking commit IDs from git log output? > > I thought of that but the adjacent commits are not candidates either. > >> > Anyway, I thought I'd email Jan as stable maintainer to decide what to >> > do about it. I'm removing it from my own backport list. >> >> I see no reason to apply any tmem patches to stable trees until >> tmem's status changes back to supported, or unless being urged >> by the tmem maintainer. > > Right. But I saw you applied it to all trees already... Jan
diff --git a/xen/common/tmem.c b/xen/common/tmem.c index 02e7e2e..0a24b3f 100644 --- a/xen/common/tmem.c +++ b/xen/common/tmem.c @@ -1845,8 +1845,6 @@ static int do_tmem_destroy_pool(uint32_t pool_id) struct client *client = current->domain->tmem_client; struct tmem_pool *pool; - if ( client->pools == NULL ) - return 0; if ( pool_id >= MAX_POOLS_PER_DOMAIN ) return 0; if ( (pool = client->pools[pool_id]) == NULL )
do_tmem_destroy_pool is checking if pools == NULL. But, pools is a fixed array. Clang 3.5 will fail to compile xen/common/tmem.c with the following error: tmem.c:1848:18: error: comparison of array 'client->pools' equal to a null pointer is always false [-Werror,-Wtautological-pointer-compare] if ( client->pools == NULL ) Signed-off-by: Julien Grall <julien.grall@linaro.org> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> --- xen/common/tmem.c | 2 -- 1 file changed, 2 deletions(-)