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

Re: [PATCH 1/3] xen/keyhandler: add missing noreturn attribute


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>
  • Date: Thu, 05 Jun 2025 14:22:02 +0200
  • Arc-authentication-results: i=1; bugseng.com; arc=none smtp.remote-ip=162.55.131.47
  • Arc-message-signature: i=1; d=bugseng.com; s=openarc; a=rsa-sha256; c=relaxed/relaxed; t=1749126122; h=DKIM-Signature:MIME-Version:Date:From:To:Cc:Subject:In-Reply-To: References:Message-ID:X-Sender:Organization:Content-Type: Content-Transfer-Encoding; bh=arKwn2jSHhIDgNuO6bTEQtIBxWHLavABUBKp05SAwhk=; b=NbvXZYXMmN8cArXeO0HKK//wKkltklyRLq+xcW0Rmgp4/UxuHYTiSP4eQcSnw5O01p9Q 4X6WQmRnBIVy9jBkQ6jG1vzg1eM6pmCJMka6gRzJqo6rtpOwWjlxFQ6Pdo4wSbT97xUA0 AgSuvIfj1dKWSk8kMkBZzYoCrtj7IDsXpReLlWEO9oKchQCafbvr7cI4jfHRBf5Wwc9ww jfOnmoGAYt2+8L/ELJxRtqXD60IEQ9CihNDBw1Mk6suNz7A9w3HLqT/8+9eRJOS3wummy /fRhohEvxsvs3BNkY3rTtNRHQ5MaS0rOCWW7pv+N2V6i8mhMHG8v4if90H0U+qnmQfABt WevhbN6s78ffdYsA8pAgq5eucAfRQ7iECHidiIsyptTJRar9UoZ6dGSgb0wh+0UHQJ+kv 0dg1DH76eMLxdwkGvtvBhQSes245NPUgNsH1CB+K+DauldpHgtc8W2FgKUkWelgqPZWby Z6kOhSI0/gahT5RBveois9vXJ6h7CFlYR1zzmZ1MjGvhbQkWVtDu34/3gdaZgFX5soQWy 8g9HSJ5dQ3TLY3A3cCOZBDQgeahZTsa+FGSaEG+VmcaXkPDVzJ6NXOuLr9BhfiEJtK0yU kiXoxoDAAF24Ru51QDs4pMSc+VaYeN4BCvlCO3ckAjhXP2pwYKEE8keAIjVW3gE=
  • Arc-seal: i=1; d=bugseng.com; s=openarc; a=rsa-sha256; cv=none; t=1749126122; b=oDKKHZNs4FijA0w/G4dB7gG5TSQdDLheCysK8IauhThrNrbJcMO0z9/VHqxt5Jsk1jDb cq9o5dUbSrinJi7RriNIIESQHWY9VyOTPRtSWk1i+xDBiXALwVqTdMVLoZsDePTtSb1Ym 7BTPbGH7fpXUglOUpdd7uUOx4XfdL8o/C2g8DEwR43GY9l9AJtZhuCAw4+VhqKc7NMKXw cyQwPqILNsv+NZOKJc7Hm3qp4kzj5hphgJhRlngP866ZWwrW9qwEAI3LNVqaCLmBeYaD5 1/YAvJOe1Pvj9Qki0FUjkZQULcXr5rOtP464WrfI54IIfWz74cMfmoVUlfEy5f8mBNHku U9s3glglAKyaQ/yp2fZ8QRQ3cHnwVH5JzhmRDsfDLMT7Dk4SVKC5MHZ8vimQV4719GuZR Raa9UmaLvMBjKSkBzR1I2QV8aaXCupUlSFZU/mw6PcK4EDs+3cDL9FPbchKHmHsAWXZSw PwhJQZru4yNqN2yviZpAwqinT0Mji9e4YGBy2iZTrtbhw6jnxjVsNe6R2PDHpLUr+GlbH JDreaYesG+Het98ekI/heCDWNSul65Rq3DnpxtK+53FlEKDPuWFHhDbqz/vi9jkA2UMTo wUrRJVP7sswz6UGJx7sb3zE5+JdaGKMADrnPV6fLah79RNoBzeZKXChgRih2ibU=
  • Authentication-results: bugseng.com; arc=none smtp.remote-ip=162.55.131.47
  • Cc: victorm.lira@xxxxxxx, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Anthony PERARD <anthony.perard@xxxxxxxxxx>, Michal Orzel <michal.orzel@xxxxxxx>, Julien Grall <julien@xxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Federico Serafini <federico.serafini@xxxxxxxxxxx>, Bertrand Marquis <bertrand.marquis@xxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • Delivery-date: Thu, 05 Jun 2025 12:22:12 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 2025-06-05 09:17, Jan Beulich wrote:
On 05.06.2025 01:49, victorm.lira@xxxxxxx wrote:
From: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>

Function `reboot_machine' does not return, but lacks the `noreturn' attribute, therefore causing a violation of MISRA C Rule 2.1: "A project shall not contain
unreachable code".

Is this (uniformly) true? Looking at ...

--- a/xen/common/keyhandler.c
+++ b/xen/common/keyhandler.c
@@ -251,7 +251,7 @@ static void cf_check dump_hwdom_registers(unsigned char key)
     }
 }

-static void cf_check reboot_machine(unsigned char key, bool unused)
+static void noreturn cf_check reboot_machine(unsigned char key, bool unused)
 {
     printk("'%c' pressed -> rebooting machine\n", key);
     machine_restart(0);

... generated code here, I can see that the compiler is perfectly able to
leverage the noreturn that machine_restart() has, resulting in no
unreachable code to be generated. That is - neither in source nor in
binary there is any unreachable code. Therefore I'm having a hard time
seeing what the violation is here.

That said, I certainly don't mind the addition of the (seemingly) missing attribute. Otoh I wonder whether an attribute the removal of which has no effect wouldn't count as "dead code" or alike, violating some other rule.


Inlining does not play a role in this case. Here reboot_machine() is marked as a violation because machine_restart() is noreturn and there is no other path upon which reboot_machine() may return, hence any function calling reboot_machine() could have portions that are inadvertently unreachable (as in never executed due to divergence) by not having the annotation. That said, in such trivial cases compilers are typically able to derive the property automatically, but they are not obliged to and, more importantly, the behavior may even differ with the same compiler using different optimization levels.

--
Nicola Vetrini, B.Sc.
Software Engineer
BUGSENG (https://bugseng.com)
LinkedIn: https://www.linkedin.com/in/nicola-vetrini-a42471253



 


Rackspace

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