[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


  • To: Kunwu Chan <chentao@xxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx, kernel-janitors@xxxxxxxxxxxxxxx
  • From: Markus Elfring <Markus.Elfring@xxxxxx>
  • Date: Wed, 17 Jan 2024 11:40:09 +0100
  • Cc: LKML <linux-kernel@xxxxxxxxxxxxxxx>, kernel test robot <lkp@xxxxxxxxx>, Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>, Borislav Petkov <bp@xxxxxxxxx>, Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>, "H. Peter Anvin" <hpa@xxxxxxxxx>, Ingo Molnar <mingo@xxxxxxxxxx>, Jürgen Groß <jgross@xxxxxxxx>, Thomas Gleixner <tglx@xxxxxxxxxxxxx>, x86@xxxxxxxxxx
  • Delivery-date: Wed, 17 Jan 2024 10:40:43 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Ui-outboundreport: notjunk:1;M01:P0:YvfY7JRbdYE=;mP4N+bwOpR1xsAuYUabDW0+JE/C AE3sP6atgL4o5w8kpHZI1+fNDuODzzspTSsZR7Ts2xw+W8gyXyG1AU9oeO8z2bx51b+YDZxEc MbwFGMGq2pX3mvIDaFzbYKAXE/lHYKT/46vU02RCllriNMVeBdckIzK5GJnmy5BhwV9obgmBU uMnG0nxdznUdK5vArhi41S8rswOBBjA+p5BXIMTwuElcMviIZDzAgwaafC7M3oRl7yOEhXGTj kl387WORzaxMA5yuQk0SH089zXvB0kDj25tXAy50rOLWbGnPMeoU+UBwkwuMUrPR+fLAckcAQ 5iezZjgPUvQOJrDOCGqLBm3U0KcSfDlOjBB/zGrIPGKsZ/USoc22ulDNOmT+q7G5NlWoHU2/S Q2jcDKj+XSwagdZr3XoaA+ntcEd90yizsoxwOHIdbnKxrYJ2gjeGocUOh1468GgeBXSIBNGB2 Hc50BeWZWY+7fO7z7MBIYuWNPJTMhx/XS5uYn0x/8yLmq4WZFNc4mMRi4VkGrPHlpi42INqOj Iy+FP0U7hHYRqyu5biKL3OMOaiuhvlWKO5uTYiPRinKgQW9YZbbe12zJq4A6Pt+LM1EtpfBfm 2r+23QPpCOME0xqwxvqai/69Nhxpxk9+MIVgx4qcnj+YfvhNimWMcfCBBO1s1EtwBmRVEUpgO SmQLEwYWfc2cV5bYz1T5thQLt2Noez5tEFErKazzhFyWhoi6R6jbgiAA7IXO3arCEQ9jxYMDF W5WS9MZKEiWQ99oQBTjVxcJ1AVBiX3Z+olPFFpLww+Dn/e2q74nF6G/dUQcFSKqFEdWHgJYOe ViGCTUON4npiWlzfq/oXAYoWsPa+0AnfUc7bevojJ/udgrqE89563O9spHpPWxCl8KHVFlm4Y wfzUoxVF2hfAH6Siuqg7QOkBgCXbJbZKUlgqEx5fjzd8GRqpuO4HdA0YWzg7NHcsdBMym5Xz4 iEsT4g==

> 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?


>       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”?

Regards,
Markus



 


Rackspace

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