|
Re: MISRA
Mitsis, Peter
I figure that that the MISRA-C changes are an attempt to step in the direction to having a version of Zephyr that is or can be certified.
toggle quoted messageShow quoted text
I've been involved in the certification process (DO-178C, though there are others) for other embedded OSes in the past and they have admittedly been beasts of process. One of the early steps that we had to go through was ensuring coding standard compliance. There is no doubt that it was both time consuming and tedious and sometimes led to code looking a little weird for compliance purposes. Despite that, the benefits to doing so (in the context of certification) were substantial as it helped to open up new business opportunities. Similarly, I suspect one of the longer term goals here is to help open up new Zephyr adoption opportunities. I realize that the above does not directly address the points of contention that you raised. By the time I am chiming in on this, I believe that others have already responded to them. My primary point that I am trying to draw attention to is how this could fit into a larger beneficial picture, despite the pain (in what is hoped to be "short-term"). Peter
-----Original Message-----
From: devel@... <devel@...> On Behalf Of Nicolas Pitre Sent: Saturday, December 11, 2021 5:01 PM To: devel@...; Kevin Hilman <khilman@...> Subject: [Zephyr-devel] MISRA <rant mode on> Could someone tell me what is all this MISRA compliance eagerness for? I can't believe that no one rebelled against that yet. I just looked at a proposed PR and this makes me want to cry. Some of the changes are good, like stricter boolean usage. But the majority of the changes are pure code obfuscation! Who in its right mind will believe that sprinkling typecasts around would make the code any safer and understandable? And that's not mentioning those _double_ typecasts being added too!!! We should instead aim for cast _reduction_ to let the compiler warn more about type mismatches and air the code, not add new ones! Note: Sometimes double casts are necessary, but we do hide them behind special accessors. Why do we have to make all loop index vars unsigned? It is a very common idiom to test for "i < 0" in loops. Making some signed and some unsigned is just adding to the inconsistency for no gain. Why do we need to test against an unsigned zero everywhere? Does anyone know a about a compiler implementation where a signed 0 vs an unsigned 0U makes any difference? And what is that obsession about qualifying every literal values anyway! When signed and unsigned expressions are combined, the result is promoted to unsigned already. So I do ask you frankly which of the following is the safest and most intuitive: #define FOO 5 #define BAR 5U [ ... code far away ... ] bool foo(int x) { return (x < FOO); } bool bar(int x) { return (x < BAR); } Notice how foo(-1) and bar(-1) don't give the same answer. Try it if you don't believe me. Whereas: bool foo(unsigned int x) { return (x < FOO); } bool bar(unsigned int x) { return (x < BAR); } Here foo(UINT_MAX) and bar(UINT_MAX) both do give the proper result. So qualifying literals everywhere when it is not absolutely required is actually evil and opens the possibility for *more* bugs not less. And why casting pointer difference to a size_t? The official type for a pointer difference is ptrdiff_t and the standard offers no explicit guarantee that sizeof(ptrdiff_t) equals sizeof(size_t). People who live by the standard will tell you that size_t _could_ be smaller and therefore silently truncate the result. What's the point of casting to a char before assigning to a char when the cast is implicit already? And a real kicker is this change: - * @return 0 on success, nonzero on failure + * @return false on success, true on failure Who really thinks that the above is idiomatic and an improvement in code understandability? It is not that MISRA does require it in this particular case, but it certainly did skew the mindset of the person who did that change. Etc. Etc. <rant mode off> So I really do challenge if MISRA compliance is a good thing for the project. Some of it certainly makes sense. Some of it is really questionable and should be excluded. I do guess (even if I don't understand why in the end) that some Zephyr users do require MISRA compliance for their project. I however do believe that this is not beneficial for the Zephyr community as a whole, and that the cost of MISRA compliance and its side effects should not be imposed on everyone without further scrutiny and discrimination. Static analysis tools have come a long way and they are far more useful, realistic and relevant when it comes to code validity and correctness at this point. Many embedded libraries and SDK's out there already maintain a MISRA compliance exception list. I think this would be a sane thing to do and that wouldn't be a first. Nicolas
|
|
|
|
Now: Zephyr: Toolchain Working Group - 12/13/2021
#cal-notice
devel@lists.zephyrproject.org Calendar <noreply@...>
Zephyr: Toolchain Working Group When: Where: Organizer: Torsten Rasmussen Description: Live meeting minutes: https://docs.google.com/document/d/1IQKBK-GcJNZG0O9QArqYfvb6Huk5xHscN-XIGEZr-z8/edit#heading=h.x36xe8bnwr9r
________________________________________________________________________________
+1 321-558-6518 United States, Orlando (Toll)
Conference ID: 682 738 030#
Local numbers | Reset PIN | Learn more about Teams | Meeting options
|
|
|
|
Event: Zephyr: Toolchain Working Group - 12/13/2021
#cal-reminder
devel@lists.zephyrproject.org Calendar <noreply@...>
Reminder: Zephyr: Toolchain Working Group When: Where: Organizer: Torsten Rasmussen An RSVP is requested. Click here to RSVP Description: Live meeting minutes: https://docs.google.com/document/d/1IQKBK-GcJNZG0O9QArqYfvb6Huk5xHscN-XIGEZr-z8/edit#heading=h.x36xe8bnwr9r
________________________________________________________________________________
+1 321-558-6518 United States, Orlando (Toll)
Conference ID: 682 738 030#
Local numbers | Reset PIN | Learn more about Teams | Meeting options
|
|
|
|
Execute vendor-specific commands to initialize BT Controller
Nazar.Palamar@...
Hello Devel team, My name is Nazar. I am working for Infineon in the SW integration team. Currently, I am working on the integration of our CYW43xxx connectivity device into Zephyr (PSoC 6 family).
{ return 0; }
{ struct net_buf *rsp; int err;
/* Execute vendor-specific commands to initialize the controller */ err = bt_hci_vendor_init_sequence(); if (err) { return err; } ... } My integration part will implement bt_hci_transport_setup and bt_hci_vendor_init_sequence functions.
|
|
|
|
Re: MISRA
Nicolas Pitre
On Sun, 12 Dec 2021, Abramo Bagnara wrote:
That's certainly better. And this provides you with the opportunity toWho in its right mind will believe that sprinkling typecasts aroundIn the specific, double casts are needed in two situations: document the reason for such a macro alongside its definition. 2) A narrower computation is assigned to a wider or differentlyWell, you aren't preventing any wrapping by adding a bunch of casts. Of course, if we want an unsigned 64-bit addition, we should writeOr even: u64 = u32_a; u64 += u32_b; Two lines, but still more enjoyable to read. Again, it is possible to encapsulate the cast used to express theOnly if that was the actual intent. I doubt such thing was deliberate. Most likely a value wrap is just not a possible outcome ever in a given code context and no one deemed it necessary to explicitly spell such thing out. Assigning to an u64 is indeed a bit suspicious in such a case. Maybe the u64 is unnecessary to start with, and if so that's what needs fixing whereas papering over it with a cast pipeline doesn't improve the code. Here I disagree.Why do we have to make all loop index vars unsigned? It is a very commons32 < 0U and s32 < 0 makes a *lot* of difference: this is one of the s32 < 5U is wrong. s32 < 5 is OK u32 < 5U is OK u32 < 5 is OK So the unqualified literal is less cluttered, and always OK. The qualified 5U is ugly, and wrong in mixed signedness cases. So the safest thing to do, and the most convivial for developers and reviewers alike, is to _NOT_ qualify literals as unsigned by default. The signed/unsigned semantic is already carried by variables those literals are used with. We might consider deviating for signed constants whose value is unchanged byThat would be an improvement indeed. OK.And why casting pointer difference to a size_t? The official type for aActually, a standard C implementation must ensure that the maximum But still, what's the point? Just to get rid of the signed nature of it? Because it is not returning a boolean. It is returning a non-zero valueAnd a real kicker is this change:Here I really don't follow you: if the function was *already* returning on failure. That's not a boolean. The common operating system idiom in that case is to return an error code, typically negative. If the particular error reason is unnecessary then it is of course a valid simplification to return a boolean instead. But it should be true for success and false for failure which is by far the most common expectation. And then the name of the function should probably be modified as well to make it clear to users that the function is no longer following the error code return paradigm. Otherwise it is best to leave it as is. Nonetheless I think that the right mindset is that missing adherenceI think both cases should be justified carefully. Some rules might have been justified back when people wrote C code K&R style but are just an annoyance with no real benefit nowdays. I've also seen too many times a bug being introduced because the code was modified just to make it [name your favorite static analysis tool / coding standard here]-compliant. It is often the case that the code is questionnably written in the first place, and adding typecasts to it makes it worse even if the scanning tool is then happy. Nicolas
|
|
|
|
Re: MISRA
Olof Johansson
On Sat, Dec 11, 2021 at 2:01 PM Nicolas Pitre <npitre@...> wrote:
I started to question a bunch of the choices in the early PRs, but it was moot. I also got the impression that the mess they are creating now with the enormous amount of obfuscating casting can/should be fixed once they've worked through the bulk of the warnings and can scale back some of them. I _really_ hope that to be the case, and not a "well, the coding is done now, so please merge it into main as well". I worry that'll be a bad experience for everybody. I just looked at a proposed PR and this makes me want to cry.I fully agree. Once they can complete the first pass on their topic branch, I expect there to be a checkpoint with a judgment call on whether it's a net improvement to Zephyr or not, and what to do with it w.r.t. merging piece of it into the main development branches. For now, I am willing to give them the benefit of the doubt, but I do worry about the final outcome being a project that's significantly harder to work with and understand. But I will wait until they're ready for said scruity. [...] So I really do challenge if MISRA compliance is a good thing for theAgain, 100% agree, especially if we start to approach things with "every PR needs to have clean MISRA checks". I certainly worry about the overall readability and general ease of both using (reading/understanding) as well as contributing to the project. But, as I said above, I'm willing to have them do the work and then we see how things look like when it's ready. Static analysis tools have come a long way and they are far more useful,My understanding is that MISRA and correctness are two completely unrelated concepts, and MISRA is mostly about avoiding undefined behaviors in specifications (that might not be causing bugs). That, plus business motivations of having a code base that is passing all checks -- ("Our code is MISRA clean", and whatever that might do to product malfunction liabilities or other certifications, etc.). Many embedded libraries and SDK's out there already maintain a MISRAWhat would you expect that to look like? Specific rules in MISRA that we're choosing to not conform to? I think that could be a very useful discussion to have once we see how the final outcome looks like, and avoid the most obfuscating ones if we can. -Olof
|
|
|
|
Cancelled Event: Zephyr Memory Footprint - biweekly discussion - Monday, January 3, 2022
#cal-cancelled
devel@lists.zephyrproject.org Calendar <noreply@...>
Cancelled: Zephyr Memory Footprint - biweekly discussion This event has been cancelled. When: Where: Organizer: devel@... Description: ______________________________
Microsoft Teams meeting
Join on your computer or mobile app
Click here to join the meetingOr call in (audio only)
+1 321-558-6518,,546018126# United States, Orlando
______________________________
|
|
|
|
Cancelled Event: Zephyr Memory Footprint - biweekly discussion - Monday, December 20, 2021
#cal-cancelled
devel@lists.zephyrproject.org Calendar <noreply@...>
Cancelled: Zephyr Memory Footprint - biweekly discussion This event has been cancelled. When: Where: Organizer: devel@... Description: ______________________________
Microsoft Teams meeting
Join on your computer or mobile app
Click here to join the meetingOr call in (audio only)
+1 321-558-6518,,546018126# United States, Orlando
______________________________
|
|
|
|
Re: MISRA
Abramo Bagnara
Who in its right mind will believe that sprinkling typecasts aroundIn the specific, double casts are needed in two situations: 1) A char argument to ctype predicates should be first converted to unsigned char (as the C standard requires) and then converted to int to avoid mixing of signed and unsigned. Said that, nothing prevents us from using helpers that accept char argument or to embed such double casts in, e.g., #define CTYPE_CHAR(c) ((int)(unsigned char)(c)) 2) A narrower computation is assigned to a wider or differently signed result, e.g., u64 = u32_a - u32_b; In this case we should write u64 = (uint64_t)(uint32_t)(u32_a - u32_b); This is used to make it explicit that (1) the type in which the difference should be computed is uint32_t and (2) that the (possibly wrapped) result should be converted to the destination type. Without that the risk is writing something like, e.g., u64 = u32_a + u32_b; without realizing that an unwanted wrapping may happen. Of course, if we want an unsigned 64-bit addition, we should write u64 = (uint64_t)u32_a + u32_b; Again, it is possible to encapsulate the cast used to express the "deliberately done" property in a macro. Another possibility to avoid the double cast and remain MISRA compliant is to use an intermediate variable: u32_c = u32_a - u32_b; u64 = u32_c; but I find that is not so nice in some situations and it does not fit well in macros. Why do we have to make all loop index vars unsigned? It is a very commons32 < 0U and s32 < 0 makes a *lot* of difference: this is one of the reasons why MISRA insists so much to not mix signed and unsigned. And what is that obsession about qualifying every literal values anyway!This is exactly the reason why signed and unsigned should not be mixed. The mistakes above would be marked as non-compliant by any MISRA checker and would not go unnoticed (differently from the situation where such mixing is spread in the code). Whereas:We might consider deviating for signed constants whose value is unchanged by promotion and usual arithmetic conversion. Would this be ok for you? And why casting pointer difference to a size_t? The official type for aActually, a standard C implementation must ensure that the maximum (positive) value for ptrdiff_t must be representable as size_t. What's the point of casting to a char before assigning to a char whenImplicit casts can (and very often do) go unnoticed. This is the very reason MISRA asks to make them explicit for the sake of code readability. [Off topic: IMHO we would also have great benefits from introducing BARR-C:2018 Rule 1.6.a, which asks that *all* non-value-preserving casts are accompanied by an explicative comment.] And a real kicker is this change:Here I really don't follow you: if the function was *already* returning a Boolean, why is this change troublesome? Said that, if the intention was *not* to return a Boolean, then the signature should not have been changed and the returned value should have been computed in a different way. The author of this change had no way to deduce the original intention with a 100% chance of success. What is needed is simply urbane feedback of the form "this function signature is deliberately returning an integer for reason X, please amend the MISRA compliance changes taking in account that." Many embedded libraries and SDK's out there already maintain a MISRAThis is definitely the intention, code quality is *always* the greater priority. Nonetheless I think that the right mindset is that missing adherence to MISRA rules should be justified carefully and not the opposite (i.e., by only following the rules that change nothing or that we are used to). This will help the whole Zephyr community: the part that needs qualifiable/certifiable code and the part that just needs better/safer code. -- Abramo Bagnara BUGSENG srl - http://bugseng.com mailto:abramo.bagnara@...
|
|
|
|
MISRA
Nicolas Pitre
<rant mode on>
Could someone tell me what is all this MISRA compliance eagerness for? I can't believe that no one rebelled against that yet. I just looked at a proposed PR and this makes me want to cry. Some of the changes are good, like stricter boolean usage. But the majority of the changes are pure code obfuscation! Who in its right mind will believe that sprinkling typecasts around would make the code any safer and understandable? And that's not mentioning those _double_ typecasts being added too!!! We should instead aim for cast _reduction_ to let the compiler warn more about type mismatches and air the code, not add new ones! Note: Sometimes double casts are necessary, but we do hide them behind special accessors. Why do we have to make all loop index vars unsigned? It is a very common idiom to test for "i < 0" in loops. Making some signed and some unsigned is just adding to the inconsistency for no gain. Why do we need to test against an unsigned zero everywhere? Does anyone know a about a compiler implementation where a signed 0 vs an unsigned 0U makes any difference? And what is that obsession about qualifying every literal values anyway! When signed and unsigned expressions are combined, the result is promoted to unsigned already. So I do ask you frankly which of the following is the safest and most intuitive: #define FOO 5 #define BAR 5U [ ... code far away ... ] bool foo(int x) { return (x < FOO); } bool bar(int x) { return (x < BAR); } Notice how foo(-1) and bar(-1) don't give the same answer. Try it if you don't believe me. Whereas: bool foo(unsigned int x) { return (x < FOO); } bool bar(unsigned int x) { return (x < BAR); } Here foo(UINT_MAX) and bar(UINT_MAX) both do give the proper result. So qualifying literals everywhere when it is not absolutely required is actually evil and opens the possibility for *more* bugs not less. And why casting pointer difference to a size_t? The official type for a pointer difference is ptrdiff_t and the standard offers no explicit guarantee that sizeof(ptrdiff_t) equals sizeof(size_t). People who live by the standard will tell you that size_t _could_ be smaller and therefore silently truncate the result. What's the point of casting to a char before assigning to a char when the cast is implicit already? And a real kicker is this change: - * @return 0 on success, nonzero on failure + * @return false on success, true on failure Who really thinks that the above is idiomatic and an improvement in code understandability? It is not that MISRA does require it in this particular case, but it certainly did skew the mindset of the person who did that change. Etc. Etc. <rant mode off> So I really do challenge if MISRA compliance is a good thing for the project. Some of it certainly makes sense. Some of it is really questionable and should be excluded. I do guess (even if I don't understand why in the end) that some Zephyr users do require MISRA compliance for their project. I however do believe that this is not beneficial for the Zephyr community as a whole, and that the cost of MISRA compliance and its side effects should not be imposed on everyone without further scrutiny and discrimination. Static analysis tools have come a long way and they are far more useful, realistic and relevant when it comes to code validity and correctness at this point. Many embedded libraries and SDK's out there already maintain a MISRA compliance exception list. I think this would be a sane thing to do and that wouldn't be a first. Nicolas
|
|
|
|
Bluetooth mesh buffers usage
Omar Morceli
Hello I have implemented a bluetooth mesh network with servel devices, and I would like to gather statistics on buffer usage, in particular outgoing and incoming messages buffers Could you tell me if there is an API I can use? Thanks
|
|
|
|
Event: Zephyr: Power Management Sync - 12/09/2021
#cal-reminder
devel@lists.zephyrproject.org Calendar <noreply@...>
Reminder: Zephyr: Power Management Sync When: Where: Organizer: devel@... An RSVP is requested. Click here to RSVP Description: ________________________________________________________________________________
Microsoft Teams meeting
Join on your computer or mobile app
Click here to join the meetingOr call in (audio only)
+1 321-558-6518,,677440320# United States, Orlando
________________________________________________________________________________
|
|
|
|
Cancelled Event: Zephyr Project: Dev Meeting - Thursday, December 9, 2021
#cal-cancelled
devel@lists.zephyrproject.org Calendar <noreply@...>
Cancelled: Zephyr Project: Dev Meeting This event has been cancelled. When: Where: Organizer: devel@... Description: ________________________________________________________________________________
+1 321-558-6518 United States, Orlando (Toll)
Conference ID: 483 314 739#
Local numbers | Reset PIN | Learn more about Teams | Meeting options
________________________________________________________________________________
|
|
|
|
Cancelled Event: Zephyr Project: Dev Meeting - Thursday, December 30, 2021
#cal-cancelled
devel@lists.zephyrproject.org Calendar <noreply@...>
Cancelled: Zephyr Project: Dev Meeting This event has been cancelled. When: Where: Organizer: devel@... Description: ________________________________________________________________________________
+1 321-558-6518 United States, Orlando (Toll)
Conference ID: 483 314 739#
Local numbers | Reset PIN | Learn more about Teams | Meeting options
________________________________________________________________________________
|
|
|
|
Cancelled Event: Zephyr Project: Dev Meeting - Thursday, December 23, 2021
#cal-cancelled
devel@lists.zephyrproject.org Calendar <noreply@...>
Cancelled: Zephyr Project: Dev Meeting This event has been cancelled. When: Where: Organizer: devel@... Description: ________________________________________________________________________________
+1 321-558-6518 United States, Orlando (Toll)
Conference ID: 483 314 739#
Local numbers | Reset PIN | Learn more about Teams | Meeting options
________________________________________________________________________________
|
|
|
|
Event: Zephyr Project: APIs - 12/07/2021
#cal-reminder
devel@lists.zephyrproject.org Calendar <noreply@...>
Reminder: Zephyr Project: APIs When: Where: Organizer: devel@... An RSVP is requested. Click here to RSVP Description: Meeting decisions/discussions in their respective PRs, tracked here: https://github.com/zephyrproject-rtos/zephyr/projects/18 ________________________________________________________________________________
+1 321-558-6518 United States, Orlando (Toll)
Conference ID: 317 990 129#
Local numbers | Reset PIN | Learn more about Teams | Meeting options
________________________________________________________________________________
|
|
|
|
Event: Zephyr: Networking Forum - 12/07/2021
#cal-reminder
devel@lists.zephyrproject.org Calendar <noreply@...>
Reminder: Zephyr: Networking Forum When: Where: Organizer: tsc@... An RSVP is requested. Click here to RSVP Description: Live Agenda/Minutes: https://docs.google.com/document/d/1qFsOpvbyLzhflJbbv4Vl__497pKHDoUCy9hjAveyCX0/edit?usp=sharing
Shared Folder: https://drive.google.com/drive/folders/1j6d0FLeOjiMil1Ellb59AsfHdzuWdAAc?usp=sharing ________________________________________________________________________________
+1 321-558-6518 United States, Orlando (Toll)
Conference ID: 458 216 365#
Local numbers | Reset PIN | Learn more about Teams | Meeting options
________________________________________________________________________________
|
|
|
|
Re: Networking Forum Agenda
The networking and release management meetings now overlap… we should consider fixing that.
David
From: devel@... <devel@...>
On Behalf Of Lubos, Robert via lists.zephyrproject.org
Hi all,
No new topics for the Networking Forum showed up, however the meeting is not cancelled per request, we’ll have some loose discussion on certain items of the networking stack.
Meeting notes: https://docs.google.com/document/d/1qFsOpvbyLzhflJbbv4Vl__497pKHDoUCy9hjAveyCX0
Shared Folder: https://drive.google.com/drive/folders/1j6d0FLeOjiMil1Ellb59AsfHdzuWdAAc?usp=sharing
Regards,
|
|
|
|
FW: Networking Forum Agenda
Lubos, Robert
Hi all,
No new topics for the Networking Forum showed up, however the meeting is not cancelled per request, we’ll have some loose discussion on certain items of the networking stack.
Meeting notes: https://docs.google.com/document/d/1qFsOpvbyLzhflJbbv4Vl__497pKHDoUCy9hjAveyCX0
Shared Folder: https://drive.google.com/drive/folders/1j6d0FLeOjiMil1Ellb59AsfHdzuWdAAc?usp=sharing
Regards,
|
|
|
|
API meeting: agenda
Henrik Brix Andersen
Hi all,
As Carles is out of office today I will be taking over todays API meeting. Agenda for today: - RFC: API Change: update LVGL from v7 to v8 - Issue: https://github.com/zephyrproject-rtos/zephyr/issues/40901 - Road to pinctrl: tracking progress - Issue: https://github.com/zephyrproject-rtos/zephyr/issues/39740 - SPI API: tracking progress - Issue: https://github.com/zephyrproject-rtos/zephyr/issues/39992 - Device init/deinit: tracking progress - Umbrella issue: https://github.com/zephyrproject-rtos/zephyr/issues/40385 If you have additional items please let me know. Teams link: https://teams.microsoft.com/l/meetup-join/19%3ameeting_NWU2MjZlYWEtZDcwMi00MWQzLTgwMjEtNDdkYjQwMjBjMmFj%40thread.v2/0?context=%7b%22Tid%22%3a%22af0096d9-700c-411a-b795-b3dd7122bad2%22%2c%22Oid%22%3a%22841a7c92-7816-4faf-9887-5e334e88f6d8%22%7d https://lists.zephyrproject.org/g/devel/calendar https://github.com/zephyrproject-rtos/zephyr/projects/18 Regards, Brix -- Henrik Brix Andersen
|
|
|
