On 2024/1/22 16:30, Dan Carpenter wrote:
On Fri, Jan 19, 2024 at 05:22:25PM +0800, Kunwu Chan wrote:
On 2024/1/17 18:40, Markus Elfring wrote:
kasprintf() returns a pointer to dynamically allocated memory
which can be NULL upon failure. Ensure the allocation was successful
by checking the pointer validity.
…
+++ b/arch/x86/xen/smp.c
@@ -61,10 +61,14 @@ void xen_smp_intr_free(unsigned int cpu)
int xen_smp_intr_init(unsigned int cpu)
{
- int rc;
+ int rc = 0;
I find the indication of a successful function execution sufficient by
the statement “return 0;” at the end.
How do you think about to omit such an extra variable initialisation?
Thanks, it's no need now. I'll remove it in v3.
This advice is good. Don't do unnecessary assignments.
Thanks for your suggestions, I'll keep it in mind.
char *resched_name, *callfunc_name, *debug_name;
resched_name = kasprintf(GFP_KERNEL, "resched%d", cpu);
+ if (!resched_name) {
+ rc = -ENOMEM;
+ goto fail;
+ }
per_cpu(xen_resched_irq, cpu).name = resched_name;
rc = bind_ipi_to_irqhandler(XEN_RESCHEDULE_VECTOR,
cpu,
You propose to apply the same error code in four if branches.
I suggest to avoid the specification of duplicate assignment statements
for this purpose.
How do you think about to use another label like “e_nomem”?
I'll add a new label to simply the code.
I'm not a Xen maintainer so I can't really comment on their style
choices. However, as one of the kernel-janitors list people, I would
say that not everyone agrees with Markus's style preferences. Markus
was banned from the list for a while, but we unbanned everyone when we
transitioned to the new list infrastructure. Do a search on lore to
find out more. https://lore.kernel.org/all/?q=Elfring
Perhaps wait for feedback from the maintainers for how to proceed?
OK, I'll wait for the feedback.
regards,
dan carpenter
--
Thanks,
Kunwu
