linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] cpu: microcode: replace 0 with NULL
@ 2019-11-26  0:27 Jules Irenge
  2019-11-26 13:53 ` Borislav Petkov
  0 siblings, 1 reply; 4+ messages in thread
From: Jules Irenge @ 2019-11-26  0:27 UTC (permalink / raw)
  To: bp, tglx; +Cc: linux-kernel, x86, hpa, mingo, Jules Irenge

Replace 0 with NULL to fix sparse tool  warning
 warning: Using plain integer as NULL pointer

Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
---
 arch/x86/kernel/cpu/microcode/amd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
index a0e52bd00ecc..4934aa7c94e7 100644
--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -418,7 +418,7 @@ static int __apply_microcode_amd(struct microcode_amd *mc)
 static bool
 apply_microcode_early_amd(u32 cpuid_1_eax, void *ucode, size_t size, bool save_patch)
 {
-	struct cont_desc desc = { 0 };
+	struct cont_desc desc = { NULL };
 	u8 (*patch)[PATCH_MAX_SIZE];
 	struct microcode_amd *mc;
 	u32 rev, dummy, *new_rev;
@@ -543,7 +543,7 @@ load_microcode_amd(bool save, u8 family, const u8 *data, size_t size);
 
 int __init save_microcode_in_initrd_amd(unsigned int cpuid_1_eax)
 {
-	struct cont_desc desc = { 0 };
+	struct cont_desc desc = { NULL };
 	enum ucode_state ret;
 	struct cpio_data cp;
 
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] cpu: microcode: replace 0 with NULL
  2019-11-26  0:27 [PATCH] cpu: microcode: replace 0 with NULL Jules Irenge
@ 2019-11-26 13:53 ` Borislav Petkov
  2019-11-26 16:03   ` Jules Irenge
  0 siblings, 1 reply; 4+ messages in thread
From: Borislav Petkov @ 2019-11-26 13:53 UTC (permalink / raw)
  To: Jules Irenge; +Cc: tglx, linux-kernel, x86, hpa, mingo

On Tue, Nov 26, 2019 at 12:27:34AM +0000, Jules Irenge wrote:
> Replace 0 with NULL to fix sparse tool  warning
>  warning: Using plain integer as NULL pointer
> 
> Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
> ---
>  arch/x86/kernel/cpu/microcode/amd.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
> index a0e52bd00ecc..4934aa7c94e7 100644
> --- a/arch/x86/kernel/cpu/microcode/amd.c
> +++ b/arch/x86/kernel/cpu/microcode/amd.c
> @@ -418,7 +418,7 @@ static int __apply_microcode_amd(struct microcode_amd *mc)
>  static bool
>  apply_microcode_early_amd(u32 cpuid_1_eax, void *ucode, size_t size, bool save_patch)
>  {
> -	struct cont_desc desc = { 0 };
> +	struct cont_desc desc = { NULL };

So my gcc guy says that 0 and NULL are equivalent as designated
initializers in this case. And if you look at the resulting asm, it
doesn't change:

# arch/x86/kernel/cpu/microcode/amd.c:421: 	struct cont_desc desc = { 0 };
	movq	$0, 8(%rsp)	#, desc
	movq	$0, (%rsp)	#, desc
	movq	$0, 16(%rsp)	#, desc
	movq	$0, 24(%rsp)	#, desc

But what I'd prefer actually is, if you do them like this:

			... = { 0,  };

because:

1. It is clear that the memory for the struct is being cleared
2. The following ones - the ones after "," are missing too, on purpose,
   because they're being cleared too.

Also pls add that explanation to the commit message.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] cpu: microcode: replace 0 with NULL
  2019-11-26 13:53 ` Borislav Petkov
@ 2019-11-26 16:03   ` Jules Irenge
  2019-11-27 21:11     ` Joe Perches
  0 siblings, 1 reply; 4+ messages in thread
From: Jules Irenge @ 2019-11-26 16:03 UTC (permalink / raw)
  To: Borislav Petkov; +Cc: Jules Irenge, tglx, linux-kernel, x86, hpa, mingo



On Tue, 26 Nov 2019, Borislav Petkov wrote:

> On Tue, Nov 26, 2019 at 12:27:34AM +0000, Jules Irenge wrote:
> > Replace 0 with NULL to fix sparse tool  warning
> >  warning: Using plain integer as NULL pointer
> > 
> > Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
> > ---
> >  arch/x86/kernel/cpu/microcode/amd.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
> > index a0e52bd00ecc..4934aa7c94e7 100644
> > --- a/arch/x86/kernel/cpu/microcode/amd.c
> > +++ b/arch/x86/kernel/cpu/microcode/amd.c
> > @@ -418,7 +418,7 @@ static int __apply_microcode_amd(struct microcode_amd *mc)
> >  static bool
> >  apply_microcode_early_amd(u32 cpuid_1_eax, void *ucode, size_t size, bool save_patch)
> >  {
> > -	struct cont_desc desc = { 0 };
> > +	struct cont_desc desc = { NULL };
> 
> So my gcc guy says that 0 and NULL are equivalent as designated
> initializers in this case. And if you look at the resulting asm, it
> doesn't change:
> 
> # arch/x86/kernel/cpu/microcode/amd.c:421: 	struct cont_desc desc = { 0 };
> 	movq	$0, 8(%rsp)	#, desc
> 	movq	$0, (%rsp)	#, desc
> 	movq	$0, 16(%rsp)	#, desc
> 	movq	$0, 24(%rsp)	#, desc
> 
> But what I'd prefer actually is, if you do them like this:
> 
> 			... = { 0,  };
> 
> because:
> 
> 1. It is clear that the memory for the struct is being cleared
> 2. The following ones - the ones after "," are missing too, on purpose,
>    because they're being cleared too.
> 
> Also pls add that explanation to the commit message.
> 
> Thx.
> 
> -- 
> Regards/Gruss,
>     Boris.
> 
> https://people.kernel.org/tglx/notes-about-netiquette
> 
Hi Boris,

Thanks for your reply and suggestion. 

I am learning patching with sparse trying to solve some problems that the 
tool complains about.

Sometime the tool is not always right. If I take your suggestion that I 
am about to do, sparse will however still complain.

so I will suggest my change to be discarded.

I will take another challenge.

Kind regards,
Jules

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] cpu: microcode: replace 0 with NULL
  2019-11-26 16:03   ` Jules Irenge
@ 2019-11-27 21:11     ` Joe Perches
  0 siblings, 0 replies; 4+ messages in thread
From: Joe Perches @ 2019-11-27 21:11 UTC (permalink / raw)
  To: Jules Irenge, Borislav Petkov; +Cc: tglx, linux-kernel, x86, hpa, mingo

On Tue, 2019-11-26 at 16:03 +0000, Jules Irenge wrote:
> 
> On Tue, 26 Nov 2019, Borislav Petkov wrote:
> 
> > On Tue, Nov 26, 2019 at 12:27:34AM +0000, Jules Irenge wrote:
> > > Replace 0 with NULL to fix sparse tool  warning
> > >  warning: Using plain integer as NULL pointer
> > > 
> > > Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
> > > ---
> > >  arch/x86/kernel/cpu/microcode/amd.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
> > > index a0e52bd00ecc..4934aa7c94e7 100644
> > > --- a/arch/x86/kernel/cpu/microcode/amd.c
> > > +++ b/arch/x86/kernel/cpu/microcode/amd.c
> > > @@ -418,7 +418,7 @@ static int __apply_microcode_amd(struct microcode_amd *mc)
> > >  static bool
> > >  apply_microcode_early_amd(u32 cpuid_1_eax, void *ucode, size_t size, bool save_patch)
> > >  {
> > > -	struct cont_desc desc = { 0 };
> > > +	struct cont_desc desc = { NULL };
> > 
> > So my gcc guy says that 0 and NULL are equivalent as designated
> > initializers in this case. And if you look at the resulting asm, it
> > doesn't change:
> > 
> > # arch/x86/kernel/cpu/microcode/amd.c:421: 	struct cont_desc desc = { 0 };
> > 	movq	$0, 8(%rsp)	#, desc
> > 	movq	$0, (%rsp)	#, desc
> > 	movq	$0, 16(%rsp)	#, desc
> > 	movq	$0, 24(%rsp)	#, desc
> > 
> > But what I'd prefer actually is, if you do them like this:
> > 
> > 			... = { 0,  };
> > 
> > because:
> > 
> > 1. It is clear that the memory for the struct is being cleared
> > 2. The following ones - the ones after "," are missing too, on purpose,
> >    because they're being cleared too.
> > 
> > Also pls add that explanation to the commit message.
> > 
> > Thx.
> > 
> > -- 
> > Regards/Gruss,
> >     Boris.
> > 
> > https://people.kernel.org/tglx/notes-about-netiquette
> > 
> Hi Boris,
> 
> Thanks for your reply and suggestion. 
> 
> I am learning patching with sparse trying to solve some problems that the 
> tool complains about.
> 
> Sometime the tool is not always right. If I take your suggestion that I 
> am about to do, sparse will however still complain.
> 
> so I will suggest my change to be discarded.
> 
> I will take another challenge.

This initializer should ether use named members with the appropriate
zeroing type or just use a blank {} so that regardless of type and
member order, the entire structure is zeroed.

	struct cont_desc desc = {};



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-11-27 21:18 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-26  0:27 [PATCH] cpu: microcode: replace 0 with NULL Jules Irenge
2019-11-26 13:53 ` Borislav Petkov
2019-11-26 16:03   ` Jules Irenge
2019-11-27 21:11     ` Joe Perches

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).