Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: Linux: Kernel

[PATCH] mm/ksm.c - Fix compile warnings (PS ignore my formatting in previous email)

 

 

Linux kernel RSS feed   Index | Next | Previous | View Threaded


askb23 at gmail

Nov 3, 2009, 10:43 PM

Post #1 of 7 (273 views)
Permalink
[PATCH] mm/ksm.c - Fix compile warnings (PS ignore my formatting in previous email)

Found the compiler warning on linux-next:

mm/ksm.c: In function ‘ksm_scan_thread’:
mm/ksm.c:1083: warning: ‘page2[0u]’ may be used uninitialized in this
function
mm/ksm.c:1083: note: ‘page2[0u]’ was declared here

fix for the above warning:

Signed-off-by: Anil SB <askb23 [at] gmail>

diff --git a/mm/ksm.c b/mm/ksm.c
index bef1af4..2ea0fd3 100644
--- a/mm/ksm.c
+++ b/mm/ksm.c
@@ -1080,7 +1080,7 @@ static void stable_tree_append(struct rmap_item *rmap_item,
*/
static void cmp_and_merge_page(struct page *page, struct rmap_item *rmap_item)
{
- struct page *page2[1];
+ struct page *page2[1] = {NULL};
struct rmap_item *tree_rmap_item;
unsigned int checksum;
int err;




--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


wharms at bfs

Nov 4, 2009, 1:57 AM

Post #2 of 7 (256 views)
Permalink
Re: [PATCH] mm/ksm.c - Fix compile warnings (PS ignore my formatting in previous email) [In reply to]

askb schrieb:
> Found the compiler warning on linux-next:
>
> mm/ksm.c: In function ‘ksm_scan_thread’:
> mm/ksm.c:1083: warning: ‘page2[0u]’ may be used uninitialized in this
> function
> mm/ksm.c:1083: note: ‘page2[0u]’ was declared here
>
> fix for the above warning:
>
> Signed-off-by: Anil SB <askb23 [at] gmail>
>
> diff --git a/mm/ksm.c b/mm/ksm.c
> index bef1af4..2ea0fd3 100644
> --- a/mm/ksm.c
> +++ b/mm/ksm.c
> @@ -1080,7 +1080,7 @@ static void stable_tree_append(struct rmap_item *rmap_item,
> */
> static void cmp_and_merge_page(struct page *page, struct rmap_item *rmap_item)
> {
> - struct page *page2[1];
> + struct page *page2[1] = {NULL};
> struct rmap_item *tree_rmap_item;
> unsigned int checksum;
> int err;
>
>

a 1 element array looks strange, did you look what kind of magic is here used ?

re,
wh

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


askb23 at gmail

Nov 4, 2009, 3:28 AM

Post #3 of 7 (250 views)
Permalink
Re: [PATCH] mm/ksm.c - Fix compile warnings (PS ignore my formatting in previous email) [In reply to]

On Wed, 2009-11-04 at 10:57 +0100, walter harms wrote:
>
> askb schrieb:
> > Found the compiler warning on linux-next:
> >
> > mm/ksm.c: In function ‘ksm_scan_thread’:
> > mm/ksm.c:1083: warning: ‘page2[0u]’ may be used uninitialized in this
> > function
> > mm/ksm.c:1083: note: ‘page2[0u]’ was declared here
> >
> > fix for the above warning:
> >
> > Signed-off-by: Anil SB <askb23 [at] gmail>
> >
> > diff --git a/mm/ksm.c b/mm/ksm.c
> > index bef1af4..2ea0fd3 100644
> > --- a/mm/ksm.c
> > +++ b/mm/ksm.c
> > @@ -1080,7 +1080,7 @@ static void stable_tree_append(struct rmap_item *rmap_item,
> > */
> > static void cmp_and_merge_page(struct page *page, struct rmap_item *rmap_item)
> > {
> > - struct page *page2[1];
> > + struct page *page2[1] = {NULL};
> > struct rmap_item *tree_rmap_item;
> > unsigned int checksum;
> > int err;
> >
> >
>
> a 1 element array looks strange, did you look what kind of magic is here used ?
>
>From my understanding, it is used for place holder to check and return
an identical page from stable_tree_search(). Can we do with a double
indirection to struct page instead of the above?
PS point me in the right direction.

Regards.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


wharms at bfs

Nov 9, 2009, 12:16 AM

Post #4 of 7 (241 views)
Permalink
Re: [PATCH] mm/ksm.c - Fix compile warnings (PS ignore my formatting in previous email) [In reply to]

askb schrieb:
> On Wed, 2009-11-04 at 10:57 +0100, walter harms wrote:
>> askb schrieb:
>>> Found the compiler warning on linux-next:
>>>
>>> mm/ksm.c: In function ‘ksm_scan_thread’:
>>> mm/ksm.c:1083: warning: ‘page2[0u]’ may be used uninitialized in this
>>> function
>>> mm/ksm.c:1083: note: ‘page2[0u]’ was declared here
>>>
>>> fix for the above warning:
>>>
>>> Signed-off-by: Anil SB <askb23 [at] gmail>
>>>
>>> diff --git a/mm/ksm.c b/mm/ksm.c
>>> index bef1af4..2ea0fd3 100644
>>> --- a/mm/ksm.c
>>> +++ b/mm/ksm.c
>>> @@ -1080,7 +1080,7 @@ static void stable_tree_append(struct rmap_item *rmap_item,
>>> */
>>> static void cmp_and_merge_page(struct page *page, struct rmap_item *rmap_item)
>>> {
>>> - struct page *page2[1];
>>> + struct page *page2[1] = {NULL};
>>> struct rmap_item *tree_rmap_item;
>>> unsigned int checksum;
>>> int err;
>>>
>>>
>> a 1 element array looks strange, did you look what kind of magic is here used ?
>>
>>From my understanding, it is used for place holder to check and return
> an identical page from stable_tree_search(). Can we do with a double
> indirection to struct page instead of the above?
> PS point me in the right direction.
>

1. sorry for delay.

2. there is no "right" direction. the result should be clear for the code, and this
may change between releases, special circumstances may occur and so on.

You may try something like that:

struct page *dst ;

function ( &dst );

IMHO that would clarify the code and fix the naming (page,page2 is not very descriptiv).


re,
wh

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


hugh.dickins at tiscali

Nov 9, 2009, 2:51 AM

Post #5 of 7 (241 views)
Permalink
Re: [PATCH] mm/ksm.c - Fix compile warnings (PS ignore my formatting in previous email) [In reply to]

On Wed, 4 Nov 2009, askb wrote:
> On Wed, 2009-11-04 at 10:57 +0100, walter harms wrote:
> >
> > askb schrieb:
> > > Found the compiler warning on linux-next:
> > >
> > > mm/ksm.c: In function ‘ksm_scan_thread’:
> > > mm/ksm.c:1083: warning: ‘page2[0u]’ may be used uninitialized in this
> > > function
> > > mm/ksm.c:1083: note: ‘page2[0u]’ was declared here
> > >
> > > fix for the above warning:
> > >
> > > Signed-off-by: Anil SB <askb23 [at] gmail>
> > >
> > > diff --git a/mm/ksm.c b/mm/ksm.c
> > > index bef1af4..2ea0fd3 100644
> > > --- a/mm/ksm.c
> > > +++ b/mm/ksm.c
> > > @@ -1080,7 +1080,7 @@ static void stable_tree_append(struct rmap_item *rmap_item,
> > > */
> > > static void cmp_and_merge_page(struct page *page, struct rmap_item *rmap_item)
> > > {
> > > - struct page *page2[1];
> > > + struct page *page2[1] = {NULL};
> > > struct rmap_item *tree_rmap_item;
> > > unsigned int checksum;
> > > int err;
> > >
> > >
> >
> > a 1 element array looks strange, did you look what kind of magic is here used ?
> >
> >From my understanding, it is used for place holder to check and return
> an identical page from stable_tree_search(). Can we do with a double
> indirection to struct page instead of the above?
> PS point me in the right direction.

I don't particularly like things like page2[1] myself, but there's
nothing actually wrong with it, so no urgency to change it.

mm/ksm.c is under active development (better Cc the people involved
with a sourcefile when considering changes to it), and I have a
collection of patches under testing, which do change some names
around here (along with more serious changes). So, thanks for looking
at this, but I'd prefer to avoid the interference of a trivial patch
at the moment - sorry.

Which version of the compiler gave you that uninitialized warning?
A version which lots of people are using, or just some snapshot?
I've never seen a warning there myself, but they do keep "refining"
that logic.

Hugh's hypothesis: for every variable x initialized by a subfunction,
there exists at least one version V of gcc, such that V reports that
x may be used uninitialized.

Hugh


askb23 at gmail

Nov 9, 2009, 5:45 AM

Post #6 of 7 (240 views)
Permalink
Re: [PATCH] mm/ksm.c - Fix compile warnings (PS ignore my formatting in previous email) [In reply to]

On Mon, 2009-11-09 at 10:51 +0000, Hugh Dickins wrote:
> On Wed, 4 Nov 2009, askb wrote:
> > On Wed, 2009-11-04 at 10:57 +0100, walter harms wrote:
> > >
> > > askb schrieb:
> > > > Found the compiler warning on linux-next:
> > > >
> > > > mm/ksm.c: In function ‘ksm_scan_thread’:
> > > > mm/ksm.c:1083: warning: ‘page2[0u]’ may be used uninitialized in this
> > > > function
> > > > mm/ksm.c:1083: note: ‘page2[0u]’ was declared here
> > > >
> > > > fix for the above warning:
> > > >
> > > > Signed-off-by: Anil SB <askb23 [at] gmail>
> > > >
> > > > diff --git a/mm/ksm.c b/mm/ksm.c
> > > > index bef1af4..2ea0fd3 100644
> > > > --- a/mm/ksm.c
> > > > +++ b/mm/ksm.c
> > > > @@ -1080,7 +1080,7 @@ static void stable_tree_append(struct rmap_item *rmap_item,
> > > > */
> > > > static void cmp_and_merge_page(struct page *page, struct rmap_item *rmap_item)
> > > > {
> > > > - struct page *page2[1];
> > > > + struct page *page2[1] = {NULL};
> > > > struct rmap_item *tree_rmap_item;
> > > > unsigned int checksum;
> > > > int err;
> > > >
> > > >
> > >
> > > a 1 element array looks strange, did you look what kind of magic is here used ?
> > >
> > >From my understanding, it is used for place holder to check and return
> > an identical page from stable_tree_search(). Can we do with a double
> > indirection to struct page instead of the above?
> > PS point me in the right direction.
>
> I don't particularly like things like page2[1] myself, but there's
> nothing actually wrong with it, so no urgency to change it.
>
> mm/ksm.c is under active development (better Cc the people involved
> with a sourcefile when considering changes to it), and I have a
> collection of patches under testing, which do change some names
> around here (along with more serious changes). So, thanks for looking
> at this, but I'd prefer to avoid the interference of a trivial patch
> at the moment - sorry.
>
> Which version of the compiler gave you that uninitialized warning?

gcc version 4.3.2 [gcc-4_3-branch revision 141291] (SUSE Linux)

> A version which lots of people are using, or just some snapshot?
> I've never seen a warning there myself, but they do keep "refining"
> that logic.
>
I got the warning while doing "make randconfig". However, now after a
cleanup and redoing the same, the warning is not seen.

> Hugh's hypothesis: for every variable x initialized by a subfunction,
> there exists at least one version V of gcc, such that V reports that
> x may be used uninitialized.
>
> Hugh

Thanks.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


hugh.dickins at tiscali

Nov 9, 2009, 7:54 AM

Post #7 of 7 (238 views)
Permalink
Re: [PATCH] mm/ksm.c - Fix compile warnings (PS ignore my formatting in previous email) [In reply to]

On Mon, 9 Nov 2009, askb wrote:
> On Mon, 2009-11-09 at 10:51 +0000, Hugh Dickins wrote:
> > On Wed, 4 Nov 2009, askb wrote:
> > > On Wed, 2009-11-04 at 10:57 +0100, walter harms wrote:
> > > > askb schrieb:
> > > > > Found the compiler warning on linux-next:
> > > > >
> > > > > mm/ksm.c: In function ‘ksm_scan_thread’:
> > > > > mm/ksm.c:1083: warning: ‘page2[0u]’ may be used uninitialized in this
> > > > > function
> > > > > mm/ksm.c:1083: note: ‘page2[0u]’ was declared here
...
> > Which version of the compiler gave you that uninitialized warning?
>
> gcc version 4.3.2 [gcc-4_3-branch revision 141291] (SUSE Linux)

Thanks: funny, that's the one I've been using most too.

>
> > A version which lots of people are using, or just some snapshot?
> > I've never seen a warning there myself, but they do keep "refining"
> > that logic.
> >
> I got the warning while doing "make randconfig". However, now after a
> cleanup and redoing the same, the warning is not seen.

I just tried the most obvious alternatives (CC_OPTIMIZE_FOR_SIZE,
OPTIMIZE_INLINING, off and on), but still didn't see the warning.

Never mind, I've shoved a NULL in my tree, but no rush for this.

Hugh

Linux kernel RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact Gossamer Threads
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.