xen/arch/arm/bootfdt.c | 2 +- xen/arch/arm/domain_build.c | 6 +++--- xen/arch/x86/cpu/microcode/core.c | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-)
Missing newline is inconsistent with the rest of the callers, since
panic() expects it.
Signed-off-by: Michal Orzel <michal.orzel@amd.com>
---
xen/arch/arm/bootfdt.c | 2 +-
xen/arch/arm/domain_build.c | 6 +++---
xen/arch/x86/cpu/microcode/core.c | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
index b6f92a174f5f..2673ad17a1e1 100644
--- a/xen/arch/arm/bootfdt.c
+++ b/xen/arch/arm/bootfdt.c
@@ -225,7 +225,7 @@ static int __init process_reserved_memory_node(const void *fdt, int node,
size_cells, data);
if ( rc == -ENOSPC )
- panic("Max number of supported reserved-memory regions reached.");
+ panic("Max number of supported reserved-memory regions reached.\n");
else if ( rc != -ENOENT )
return rc;
return 0;
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 579bc8194fed..d0d6be922db1 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -74,7 +74,7 @@ int __init parse_arch_dom0_param(const char *s, const char *e)
return 0;
#else
- panic("'sve' property found, but CONFIG_ARM64_SVE not selected");
+ panic("'sve' property found, but CONFIG_ARM64_SVE not selected\n");
#endif
}
@@ -697,7 +697,7 @@ static void __init allocate_static_memory(struct domain *d,
return;
fail:
- panic("Failed to allocate requested static memory for domain %pd.", d);
+ panic("Failed to allocate requested static memory for domain %pd.\n", d);
}
/*
@@ -769,7 +769,7 @@ static void __init assign_static_memory_11(struct domain *d,
return;
fail:
- panic("Failed to assign requested static memory for direct-map domain %pd.",
+ panic("Failed to assign requested static memory for direct-map domain %pd.\n",
d);
}
diff --git a/xen/arch/x86/cpu/microcode/core.c b/xen/arch/x86/cpu/microcode/core.c
index e65af4b82ea3..c3fee629063e 100644
--- a/xen/arch/x86/cpu/microcode/core.c
+++ b/xen/arch/x86/cpu/microcode/core.c
@@ -524,7 +524,7 @@ static int control_thread_fn(const struct microcode_patch *patch)
*/
if ( wait_for_condition(wait_cpu_callout, (done + 1),
MICROCODE_UPDATE_TIMEOUT_US) )
- panic("Timeout when finished updating microcode (finished %u/%u)",
+ panic("Timeout when finished updating microcode (finished %u/%u)\n",
done, nr_cores);
/* Print warning message once if long time is spent here */
base-commit: 2f69ef96801f0d2b9646abf6396e60f99c56e3a0
--
2.25.1
Hi Michal, > On 14 Jun 2023, at 09:30, Michal Orzel <michal.orzel@amd.com> wrote: > > Missing newline is inconsistent with the rest of the callers, since > panic() expects it. > > Signed-off-by: Michal Orzel <michal.orzel@amd.com> Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com> And I agree with Julien: trailing punctuation is not an issue and I would definitely not require you to fix it. Cheers Bertrand > --- > xen/arch/arm/bootfdt.c | 2 +- > xen/arch/arm/domain_build.c | 6 +++--- > xen/arch/x86/cpu/microcode/core.c | 2 +- > 3 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c > index b6f92a174f5f..2673ad17a1e1 100644 > --- a/xen/arch/arm/bootfdt.c > +++ b/xen/arch/arm/bootfdt.c > @@ -225,7 +225,7 @@ static int __init process_reserved_memory_node(const void *fdt, int node, > size_cells, data); > > if ( rc == -ENOSPC ) > - panic("Max number of supported reserved-memory regions reached."); > + panic("Max number of supported reserved-memory regions reached.\n"); > else if ( rc != -ENOENT ) > return rc; > return 0; > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > index 579bc8194fed..d0d6be922db1 100644 > --- a/xen/arch/arm/domain_build.c > +++ b/xen/arch/arm/domain_build.c > @@ -74,7 +74,7 @@ int __init parse_arch_dom0_param(const char *s, const char *e) > > return 0; > #else > - panic("'sve' property found, but CONFIG_ARM64_SVE not selected"); > + panic("'sve' property found, but CONFIG_ARM64_SVE not selected\n"); > #endif > } > > @@ -697,7 +697,7 @@ static void __init allocate_static_memory(struct domain *d, > return; > > fail: > - panic("Failed to allocate requested static memory for domain %pd.", d); > + panic("Failed to allocate requested static memory for domain %pd.\n", d); > } > > /* > @@ -769,7 +769,7 @@ static void __init assign_static_memory_11(struct domain *d, > return; > > fail: > - panic("Failed to assign requested static memory for direct-map domain %pd.", > + panic("Failed to assign requested static memory for direct-map domain %pd.\n", > d); > } > > diff --git a/xen/arch/x86/cpu/microcode/core.c b/xen/arch/x86/cpu/microcode/core.c > index e65af4b82ea3..c3fee629063e 100644 > --- a/xen/arch/x86/cpu/microcode/core.c > +++ b/xen/arch/x86/cpu/microcode/core.c > @@ -524,7 +524,7 @@ static int control_thread_fn(const struct microcode_patch *patch) > */ > if ( wait_for_condition(wait_cpu_callout, (done + 1), > MICROCODE_UPDATE_TIMEOUT_US) ) > - panic("Timeout when finished updating microcode (finished %u/%u)", > + panic("Timeout when finished updating microcode (finished %u/%u)\n", > done, nr_cores); > > /* Print warning message once if long time is spent here */ > > base-commit: 2f69ef96801f0d2b9646abf6396e60f99c56e3a0 > -- > 2.25.1 >
On 14/06/2023 8:30 am, Michal Orzel wrote: > Missing newline is inconsistent with the rest of the callers, since > panic() expects it. > > Signed-off-by: Michal Orzel <michal.orzel@amd.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> although... > --- > xen/arch/arm/bootfdt.c | 2 +- > xen/arch/arm/domain_build.c | 6 +++--- > xen/arch/x86/cpu/microcode/core.c | 2 +- > 3 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c > index b6f92a174f5f..2673ad17a1e1 100644 > --- a/xen/arch/arm/bootfdt.c > +++ b/xen/arch/arm/bootfdt.c > @@ -225,7 +225,7 @@ static int __init process_reserved_memory_node(const void *fdt, int node, > size_cells, data); > > if ( rc == -ENOSPC ) > - panic("Max number of supported reserved-memory regions reached."); > + panic("Max number of supported reserved-memory regions reached.\n"); Trailing punctuation like . or ! is useless. Most messages don't have them, and it just takes up space in .rodata, the console ring, and time on the UART. I'd recommend dropping the ones you modify, and/or cleaning it up more widely. ~Andrew
On 14/06/2023 10:04, Andrew Cooper wrote: > > > On 14/06/2023 8:30 am, Michal Orzel wrote: >> Missing newline is inconsistent with the rest of the callers, since >> panic() expects it. >> >> Signed-off-by: Michal Orzel <michal.orzel@amd.com> > > Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> > > although... > >> --- >> xen/arch/arm/bootfdt.c | 2 +- >> xen/arch/arm/domain_build.c | 6 +++--- >> xen/arch/x86/cpu/microcode/core.c | 2 +- >> 3 files changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c >> index b6f92a174f5f..2673ad17a1e1 100644 >> --- a/xen/arch/arm/bootfdt.c >> +++ b/xen/arch/arm/bootfdt.c >> @@ -225,7 +225,7 @@ static int __init process_reserved_memory_node(const void *fdt, int node, >> size_cells, data); >> >> if ( rc == -ENOSPC ) >> - panic("Max number of supported reserved-memory regions reached."); >> + panic("Max number of supported reserved-memory regions reached.\n"); > > Trailing punctuation like . or ! is useless. Most messages don't have > them, and it just takes up space in .rodata, the console ring, and time > on the UART. > > I'd recommend dropping the ones you modify, and/or cleaning it up more > widely. I will keep in mind to do that in global scope in the next patch. We also have quite a lot of printk() with trailing punctuation. ~Michal
On 14/06/2023 09:09, Michal Orzel wrote: > > > On 14/06/2023 10:04, Andrew Cooper wrote: >> >> >> On 14/06/2023 8:30 am, Michal Orzel wrote: >>> Missing newline is inconsistent with the rest of the callers, since >>> panic() expects it. >>> >>> Signed-off-by: Michal Orzel <michal.orzel@amd.com> >> >> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> >> >> although... >> >>> --- >>> xen/arch/arm/bootfdt.c | 2 +- >>> xen/arch/arm/domain_build.c | 6 +++--- >>> xen/arch/x86/cpu/microcode/core.c | 2 +- >>> 3 files changed, 5 insertions(+), 5 deletions(-) >>> >>> diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c >>> index b6f92a174f5f..2673ad17a1e1 100644 >>> --- a/xen/arch/arm/bootfdt.c >>> +++ b/xen/arch/arm/bootfdt.c >>> @@ -225,7 +225,7 @@ static int __init process_reserved_memory_node(const void *fdt, int node, >>> size_cells, data); >>> >>> if ( rc == -ENOSPC ) >>> - panic("Max number of supported reserved-memory regions reached."); >>> + panic("Max number of supported reserved-memory regions reached.\n"); >> >> Trailing punctuation like . or ! is useless. Most messages don't have >> them, and it just takes up space in .rodata, the console ring, and time >> on the UART. >> >> I'd recommend dropping the ones you modify, and/or cleaning it up more >> widely. > I will keep in mind to do that in global scope in the next patch. > We also have quite a lot of printk() with trailing punctuation. This is quite a bit of churn and I am unconvinced this is necessary. Also, if the others want such style, then this should be agreed on in the CODING_STYLE first. Otherwise, I am afraid this is not something I will enforce because I see limited value. Cheers, -- Julien Grall
On 14/06/2023 11:02, Julien Grall wrote: > > > On 14/06/2023 09:09, Michal Orzel wrote: >> >> >> On 14/06/2023 10:04, Andrew Cooper wrote: >>> >>> >>> On 14/06/2023 8:30 am, Michal Orzel wrote: >>>> Missing newline is inconsistent with the rest of the callers, since >>>> panic() expects it. >>>> >>>> Signed-off-by: Michal Orzel <michal.orzel@amd.com> >>> >>> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> >>> >>> although... >>> >>>> --- >>>> xen/arch/arm/bootfdt.c | 2 +- >>>> xen/arch/arm/domain_build.c | 6 +++--- >>>> xen/arch/x86/cpu/microcode/core.c | 2 +- >>>> 3 files changed, 5 insertions(+), 5 deletions(-) >>>> >>>> diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c >>>> index b6f92a174f5f..2673ad17a1e1 100644 >>>> --- a/xen/arch/arm/bootfdt.c >>>> +++ b/xen/arch/arm/bootfdt.c >>>> @@ -225,7 +225,7 @@ static int __init process_reserved_memory_node(const void *fdt, int node, >>>> size_cells, data); >>>> >>>> if ( rc == -ENOSPC ) >>>> - panic("Max number of supported reserved-memory regions reached."); >>>> + panic("Max number of supported reserved-memory regions reached.\n"); >>> >>> Trailing punctuation like . or ! is useless. Most messages don't have >>> them, and it just takes up space in .rodata, the console ring, and time >>> on the UART. >>> >>> I'd recommend dropping the ones you modify, and/or cleaning it up more >>> widely. >> I will keep in mind to do that in global scope in the next patch. >> We also have quite a lot of printk() with trailing punctuation. > > This is quite a bit of churn and I am unconvinced this is necessary. > Also, if the others want such style, then this should be agreed on in > the CODING_STYLE first. Otherwise, I am afraid this is not something I > will enforce because I see limited value. > I then suggest to take this patch as is if you are also happy with it like others. ~Michal
On 14/06/2023 10:06, Michal Orzel wrote: > > > On 14/06/2023 11:02, Julien Grall wrote: >> >> >> On 14/06/2023 09:09, Michal Orzel wrote: >>> >>> >>> On 14/06/2023 10:04, Andrew Cooper wrote: >>>> >>>> >>>> On 14/06/2023 8:30 am, Michal Orzel wrote: >>>>> Missing newline is inconsistent with the rest of the callers, since >>>>> panic() expects it. >>>>> >>>>> Signed-off-by: Michal Orzel <michal.orzel@amd.com> >>>> >>>> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> >>>> >>>> although... >>>> >>>>> --- >>>>> xen/arch/arm/bootfdt.c | 2 +- >>>>> xen/arch/arm/domain_build.c | 6 +++--- >>>>> xen/arch/x86/cpu/microcode/core.c | 2 +- >>>>> 3 files changed, 5 insertions(+), 5 deletions(-) >>>>> >>>>> diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c >>>>> index b6f92a174f5f..2673ad17a1e1 100644 >>>>> --- a/xen/arch/arm/bootfdt.c >>>>> +++ b/xen/arch/arm/bootfdt.c >>>>> @@ -225,7 +225,7 @@ static int __init process_reserved_memory_node(const void *fdt, int node, >>>>> size_cells, data); >>>>> >>>>> if ( rc == -ENOSPC ) >>>>> - panic("Max number of supported reserved-memory regions reached."); >>>>> + panic("Max number of supported reserved-memory regions reached.\n"); >>>> >>>> Trailing punctuation like . or ! is useless. Most messages don't have >>>> them, and it just takes up space in .rodata, the console ring, and time >>>> on the UART. >>>> >>>> I'd recommend dropping the ones you modify, and/or cleaning it up more >>>> widely. >>> I will keep in mind to do that in global scope in the next patch. >>> We also have quite a lot of printk() with trailing punctuation. >> >> This is quite a bit of churn and I am unconvinced this is necessary. >> Also, if the others want such style, then this should be agreed on in >> the CODING_STYLE first. Otherwise, I am afraid this is not something I >> will enforce because I see limited value. >> > I then suggest to take this patch as is if you are also happy with it like others. The patch looks fine. I will commit it a bit later just to give a chance to Bertrand/Stefano to object. Cheers, -- Julien Grall
Hi, On 14/06/2023 10:09, Julien Grall wrote: > The patch looks fine. I will commit it a bit later just to give a chance > to Bertrand/Stefano to object. And committed. Cheers, -- Julien Grall
> On 14 Jun 2023, at 08:30, Michal Orzel <michal.orzel@amd.com> wrote: > > Missing newline is inconsistent with the rest of the callers, since > panic() expects it. > > Signed-off-by: Michal Orzel <michal.orzel@amd.com> Reviewed-by: Luca Fancellu <luca.fancellu@arm.com> > --- > xen/arch/arm/bootfdt.c | 2 +- > xen/arch/arm/domain_build.c | 6 +++--- > xen/arch/x86/cpu/microcode/core.c | 2 +- > 3 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c > index b6f92a174f5f..2673ad17a1e1 100644 > --- a/xen/arch/arm/bootfdt.c > +++ b/xen/arch/arm/bootfdt.c > @@ -225,7 +225,7 @@ static int __init process_reserved_memory_node(const void *fdt, int node, > size_cells, data); > > if ( rc == -ENOSPC ) > - panic("Max number of supported reserved-memory regions reached."); > + panic("Max number of supported reserved-memory regions reached.\n"); > else if ( rc != -ENOENT ) > return rc; > return 0; > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > index 579bc8194fed..d0d6be922db1 100644 > --- a/xen/arch/arm/domain_build.c > +++ b/xen/arch/arm/domain_build.c > @@ -74,7 +74,7 @@ int __init parse_arch_dom0_param(const char *s, const char *e) > > return 0; > #else > - panic("'sve' property found, but CONFIG_ARM64_SVE not selected"); > + panic("'sve' property found, but CONFIG_ARM64_SVE not selected\n"); Sorry about that! I’ve missed it > #endif > } > > @@ -697,7 +697,7 @@ static void __init allocate_static_memory(struct domain *d, > return; > > fail: > - panic("Failed to allocate requested static memory for domain %pd.", d); > + panic("Failed to allocate requested static memory for domain %pd.\n", d); > } > > /* > @@ -769,7 +769,7 @@ static void __init assign_static_memory_11(struct domain *d, > return; > > fail: > - panic("Failed to assign requested static memory for direct-map domain %pd.", > + panic("Failed to assign requested static memory for direct-map domain %pd.\n", > d); > } > > diff --git a/xen/arch/x86/cpu/microcode/core.c b/xen/arch/x86/cpu/microcode/core.c > index e65af4b82ea3..c3fee629063e 100644 > --- a/xen/arch/x86/cpu/microcode/core.c > +++ b/xen/arch/x86/cpu/microcode/core.c > @@ -524,7 +524,7 @@ static int control_thread_fn(const struct microcode_patch *patch) > */ > if ( wait_for_condition(wait_cpu_callout, (done + 1), > MICROCODE_UPDATE_TIMEOUT_US) ) > - panic("Timeout when finished updating microcode (finished %u/%u)", > + panic("Timeout when finished updating microcode (finished %u/%u)\n", > done, nr_cores); > > /* Print warning message once if long time is spent here */ > > base-commit: 2f69ef96801f0d2b9646abf6396e60f99c56e3a0 > -- > 2.25.1 > >
On 14.06.2023 09:45, Luca Fancellu wrote: >> On 14 Jun 2023, at 08:30, Michal Orzel <michal.orzel@amd.com> wrote: >> >> Missing newline is inconsistent with the rest of the callers, since >> panic() expects it. >> >> Signed-off-by: Michal Orzel <michal.orzel@amd.com> > > Reviewed-by: Luca Fancellu <luca.fancellu@arm.com> Acked-by: Jan Beulich <jbeulich@suse.com>
© 2016 - 2024 Red Hat, Inc.