Re: Policy for return by reference values in error cases
On Thu, Mar 25, 2021, 1:16 PM Sebastian Huber < sebastian.hu...@embedded-brains.de> wrote: > On 23/03/2021 20:21, Gedare Bloom wrote: > > >> On Tue, Mar 23, 2021 at 12:58 PM Sebastian Huber< > sebastian.hu...@embedded-brains.de> wrote: > >>> On 23/03/2021 18:48, Joel Sherrill wrote: > >>> > My first thought is that I don't like covering up for applications > that do the wrong thing. > > +.5 > > > >>> This topic just came up recently in a discussion about defensive > >>> programming. We also test for NULL pointers. > > +1 > Ok, I will leave it as it is for now unless there is a strong desire to > change this. > Is this one of our small rules in the coding style? > > -- > embedded brains GmbH > Herr Sebastian HUBER > Dornierstr. 4 > 82178 Puchheim > Germany > email: sebastian.hu...@embedded-brains.de > phone: +49-89-18 94 741 - 16 > fax: +49-89-18 94 741 - 08 > > Registergericht: Amtsgericht München > Registernummer: HRB 157899 > Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler > Unsere Datenschutzerklärung finden Sie hier: > https://embedded-brains.de/datenschutzerklaerung/ > > ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: Policy for return by reference values in error cases
On 23/03/2021 20:21, Gedare Bloom wrote: On Tue, Mar 23, 2021 at 12:58 PM Sebastian Huber wrote: On 23/03/2021 18:48, Joel Sherrill wrote: My first thought is that I don't like covering up for applications that do the wrong thing. +.5 This topic just came up recently in a discussion about defensive programming. We also test for NULL pointers. +1 Ok, I will leave it as it is for now unless there is a strong desire to change this. -- embedded brains GmbH Herr Sebastian HUBER Dornierstr. 4 82178 Puchheim Germany email: sebastian.hu...@embedded-brains.de phone: +49-89-18 94 741 - 16 fax: +49-89-18 94 741 - 08 Registergericht: Amtsgericht München Registernummer: HRB 157899 Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler Unsere Datenschutzerklärung finden Sie hier: https://embedded-brains.de/datenschutzerklaerung/ ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: Policy for return by reference values in error cases
On Tue, Mar 23, 2021 at 1:21 PM Gedare Bloom wrote:
>
> On Tue, Mar 23, 2021 at 12:37 PM Joel Sherrill wrote:
> >
> >
> >
> > On Tue, Mar 23, 2021 at 12:58 PM Sebastian Huber
> > wrote:
> >>
> >> On 23/03/2021 18:48, Joel Sherrill wrote:
> >>
> >> > My first thought is that I don't like covering up for applications
> >> > that do the wrong thing.
> +.5
>
> >> This topic just came up recently in a discussion about defensive
> >> programming. We also test for NULL pointers.
> +1
>
> >> >
> >> > I'm overall rather ambiguous. It is possible that setting the value at
> >> > the top of the function could lead to overridden before used issues
> >> > with warnings and static analysis.
> >>
> >> You mean code like this:
> >>
> >> void (int *x, int y)
> >>
> >> {
> >>
> >>*x = 0;
> >>
> >> if (y) {
> >>
> >> *x = 1;
> >>
> >> } else {
> >>
> >> *x = 2;
> >>
> >> }
> >>
> >> ?
> >
> >
> > Yep. That's a pretty clear case.
> >
> > Others should speak up but I just don't want the solution pattern
> > to introduce warnings or static analysis reports. It easily could.
> >
>
> Generally, the error-producing path should allow the static analysis
> to find that the value gets set. In fact, I'm a bit surprised the
> static analysis doesn't complain about the original problem, that some
> variable can be used uninitialized when for example
> rtems_event_receive() returns before updating its pointer argument.
> Probably, the points-to analysis is complicated to find this case, but
> it seems like a good case to reduce and bring to some expert in static
> analysis.
>
I guess we don't have this kind of bad example in our testsuite though. :)
> So I don't see a fundamental problem with a pattern that initializes
> these out parameters at the top of the function to a default value.
>
> >
> >>
> >> > I don't want to see every error case assign a value to an output
> >> > parameter though.
> >> Yes, I don't like this also.
> >
> >
> > I have my own wish list for error paths eventually if we ever get bored. :)
> >
> >
> > --joel
> >
> >>
> >>
> >> --
> >> embedded brains GmbH
> >> Herr Sebastian HUBER
> >> Dornierstr. 4
> >> 82178 Puchheim
> >> Germany
> >> email: sebastian.hu...@embedded-brains.de
> >> phone: +49-89-18 94 741 - 16
> >> fax: +49-89-18 94 741 - 08
> >>
> >> Registergericht: Amtsgericht München
> >> Registernummer: HRB 157899
> >> Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
> >> Unsere Datenschutzerklärung finden Sie hier:
> >> https://embedded-brains.de/datenschutzerklaerung/
> >>
> > ___
> > devel mailing list
> > devel@rtems.org
> > http://lists.rtems.org/mailman/listinfo/devel
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
Re: Policy for return by reference values in error cases
On Tue, Mar 23, 2021 at 12:37 PM Joel Sherrill wrote:
>
>
>
> On Tue, Mar 23, 2021 at 12:58 PM Sebastian Huber
> wrote:
>>
>> On 23/03/2021 18:48, Joel Sherrill wrote:
>>
>> > My first thought is that I don't like covering up for applications
>> > that do the wrong thing.
+.5
>> This topic just came up recently in a discussion about defensive
>> programming. We also test for NULL pointers.
+1
>> >
>> > I'm overall rather ambiguous. It is possible that setting the value at
>> > the top of the function could lead to overridden before used issues
>> > with warnings and static analysis.
>>
>> You mean code like this:
>>
>> void (int *x, int y)
>>
>> {
>>
>>*x = 0;
>>
>> if (y) {
>>
>> *x = 1;
>>
>> } else {
>>
>> *x = 2;
>>
>> }
>>
>> ?
>
>
> Yep. That's a pretty clear case.
>
> Others should speak up but I just don't want the solution pattern
> to introduce warnings or static analysis reports. It easily could.
>
Generally, the error-producing path should allow the static analysis
to find that the value gets set. In fact, I'm a bit surprised the
static analysis doesn't complain about the original problem, that some
variable can be used uninitialized when for example
rtems_event_receive() returns before updating its pointer argument.
Probably, the points-to analysis is complicated to find this case, but
it seems like a good case to reduce and bring to some expert in static
analysis.
So I don't see a fundamental problem with a pattern that initializes
these out parameters at the top of the function to a default value.
>
>>
>> > I don't want to see every error case assign a value to an output
>> > parameter though.
>> Yes, I don't like this also.
>
>
> I have my own wish list for error paths eventually if we ever get bored. :)
>
>
> --joel
>
>>
>>
>> --
>> embedded brains GmbH
>> Herr Sebastian HUBER
>> Dornierstr. 4
>> 82178 Puchheim
>> Germany
>> email: sebastian.hu...@embedded-brains.de
>> phone: +49-89-18 94 741 - 16
>> fax: +49-89-18 94 741 - 08
>>
>> Registergericht: Amtsgericht München
>> Registernummer: HRB 157899
>> Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
>> Unsere Datenschutzerklärung finden Sie hier:
>> https://embedded-brains.de/datenschutzerklaerung/
>>
> ___
> devel mailing list
> devel@rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
Re: Policy for return by reference values in error cases
On Tue, Mar 23, 2021 at 12:58 PM Sebastian Huber <
sebastian.hu...@embedded-brains.de> wrote:
> On 23/03/2021 18:48, Joel Sherrill wrote:
>
> > My first thought is that I don't like covering up for applications
> > that do the wrong thing.
> This topic just came up recently in a discussion about defensive
> programming. We also test for NULL pointers.
> >
> > I'm overall rather ambiguous. It is possible that setting the value at
> > the top of the function could lead to overridden before used issues
> > with warnings and static analysis.
>
> You mean code like this:
>
> void (int *x, int y)
>
> {
>
>*x = 0;
>
> if (y) {
>
> *x = 1;
>
> } else {
>
> *x = 2;
>
> }
>
> ?
>
Yep. That's a pretty clear case.
Others should speak up but I just don't want the solution pattern
to introduce warnings or static analysis reports. It easily could.
> > I don't want to see every error case assign a value to an output
> > parameter though.
> Yes, I don't like this also.
>
I have my own wish list for error paths eventually if we ever get bored. :)
--joel
>
> --
> embedded brains GmbH
> Herr Sebastian HUBER
> Dornierstr. 4
> 82178 Puchheim
> Germany
> email: sebastian.hu...@embedded-brains.de
> phone: +49-89-18 94 741 - 16
> fax: +49-89-18 94 741 - 08
>
> Registergericht: Amtsgericht München
> Registernummer: HRB 157899
> Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
> Unsere Datenschutzerklärung finden Sie hier:
> https://embedded-brains.de/datenschutzerklaerung/
>
>
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
Re: Policy for return by reference values in error cases
On 23/03/2021 18:48, Joel Sherrill wrote:
My first thought is that I don't like covering up for applications
that do the wrong thing.
This topic just came up recently in a discussion about defensive
programming. We also test for NULL pointers.
I'm overall rather ambiguous. It is possible that setting the value at
the top of the function could lead to overridden before used issues
with warnings and static analysis.
You mean code like this:
void (int *x, int y)
{
*x = 0;
if (y) {
*x = 1;
} else {
*x = 2;
}
?
I don't want to see every error case assign a value to an output
parameter though.
Yes, I don't like this also.
--
embedded brains GmbH
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.hu...@embedded-brains.de
phone: +49-89-18 94 741 - 16
fax: +49-89-18 94 741 - 08
Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
Re: Policy for return by reference values in error cases
On Tue, Mar 23, 2021, 12:30 PM Sebastian Huber <
sebastian.hu...@embedded-brains.de> wrote:
> Hello,
>
> for the RTEMS pre-qualification project I review currently most parts of
> the Classic API. It seems we have an unwritten rule that directives
> should not write to variables referenced by directive parameters if an
> error occurs. Examples:
>
> * rtems_task_mode(previous_mode)
>
> * rtems_task_set_priority(previous_priority)
>
> * rtems_message_queue_receive(message_size)
>
> * rtems_event_receive(received_events)
>
> We should think about changing this rule to instead return "safe" values
> in error conditions. For example, lets have an application bug I
> encountered this year. The code originally looked like this:
>
> #define APP_TIMEOUT RTEMS_NO_TIMEOUT
>
> rtems_event_set events;
>
> (void) rtems_event_receive(
>
> RTEMS_ALL_EVENTS,
> RTEMS_EVENT_ANY | RTEMS_WAIT,
> APP_TIMEOUT,
>
>);
>
> if (event & X) != 0) {
>
> ...
>
> }
>
> Ok, you should check the return value, but ...
>
> Something didn't go well and someone changed APP_TIMEOUT:
>
> #define APP_TIMEOUT 1000
>
> Now, sometimes the rtems_event_receive() timed out and the application
> worked with events produced by the stack frame and not
> rtems_event_receive(). In the case of rtems_message_queue_receive() it
> would be the message size.
>
> In order to let broken applications fail a bit more gracefully, it would
> help to set the events and the message size to zero if the directive
> returns with an error. The modes could be set to the defaults, the
> priority to some invalid value, etc.
>
> What do you think?
>
My first thought is that I don't like covering up for applications that do
the wrong thing.
I'm overall rather ambiguous. It is possible that setting the value at the
top of the function could lead to overridden before used issues with
warnings and static analysis. I don't want to see every error case assign a
value to an output parameter though.
--joel
> --
> embedded brains GmbH
> Herr Sebastian HUBER
> Dornierstr. 4
> 82178 Puchheim
> Germany
> email: sebastian.hu...@embedded-brains.de
> phone: +49-89-18 94 741 - 16
> fax: +49-89-18 94 741 - 08
>
> Registergericht: Amtsgericht München
> Registernummer: HRB 157899
> Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
> Unsere Datenschutzerklärung finden Sie hier:
> https://embedded-brains.de/datenschutzerklaerung/
>
> ___
> devel mailing list
> devel@rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
Policy for return by reference values in error cases
Hello,
for the RTEMS pre-qualification project I review currently most parts of
the Classic API. It seems we have an unwritten rule that directives
should not write to variables referenced by directive parameters if an
error occurs. Examples:
* rtems_task_mode(previous_mode)
* rtems_task_set_priority(previous_priority)
* rtems_message_queue_receive(message_size)
* rtems_event_receive(received_events)
We should think about changing this rule to instead return "safe" values
in error conditions. For example, lets have an application bug I
encountered this year. The code originally looked like this:
#define APP_TIMEOUT RTEMS_NO_TIMEOUT
rtems_event_set events;
(void) rtems_event_receive(
RTEMS_ALL_EVENTS,
RTEMS_EVENT_ANY | RTEMS_WAIT,
APP_TIMEOUT,
);
if (event & X) != 0) {
...
}
Ok, you should check the return value, but ...
Something didn't go well and someone changed APP_TIMEOUT:
#define APP_TIMEOUT 1000
Now, sometimes the rtems_event_receive() timed out and the application
worked with events produced by the stack frame and not
rtems_event_receive(). In the case of rtems_message_queue_receive() it
would be the message size.
In order to let broken applications fail a bit more gracefully, it would
help to set the events and the message size to zero if the directive
returns with an error. The modes could be set to the defaults, the
priority to some invalid value, etc.
What do you think?
--
embedded brains GmbH
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.hu...@embedded-brains.de
phone: +49-89-18 94 741 - 16
fax: +49-89-18 94 741 - 08
Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
