[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2] x86/xen: Add some null pointer checking to smp.c



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




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.