Skip to content
This repository has been archived by the owner on Nov 11, 2024. It is now read-only.

Fix for uGnssPosGetStreamedStart: Return zero on success #153

Merged
merged 1 commit into from
Oct 31, 2023

Conversation

alonbl
Copy link
Contributor

@alonbl alonbl commented Oct 31, 2023

per documentation zero should be returned when success while nested uGnssMsgPrivateReceiveStart returns a handle.

 * @return               zero on success or negative error code on
 *                       failure.

per documentation zero should be returned when success while nested
uGnssMsgPrivateReceiveStart returns a handle.

 * @return               zero on success or negative error code on
 *                       failure.

Signed-off-by: Alon Bar-Lev <[email protected]>
@alonbl
Copy link
Contributor Author

alonbl commented Oct 31, 2023

Hi @RobMeades,

I think this library should be rewritten from scratch with the cleanup pattern.

    if (whatever_fail) {
       goto cleanup;
    }

cleanup:

    if (x != NULL) {
        release;
    }

the nesting and resource management in each branch hides so many bugs... it already took me two days to trace these into the library... two days of mine == ~100 units of ublox... :)

@alonbl alonbl closed this Oct 31, 2023
@alonbl alonbl reopened this Oct 31, 2023
@RobMeades
Copy link
Contributor

two days of mine == ~100 units of ublox... :)

Surely you're not that cheap ;-). Or are we reassuringly expensive? Anyway, you make a good point, especially when the functions get rather out of hand as that one does.

Thanks again, will test and merge.

@RobMeades RobMeades self-requested a review October 31, 2023 19:20
Copy link
Contributor

@RobMeades RobMeades left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test system says "yo".

@RobMeades RobMeades merged commit cd01364 into u-blox:master Oct 31, 2023
2 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants