search

awslabs / aws-c-mqtt

C99 implementation of the MQTT 3.1.1 specification.
Apache License 2.0
93 stars 30 forks source link

Raise error on null mqtt5 client #369

Closed xiazhvera closed 3 months ago

xiazhvera commented 4 months ago

Issue #, if available:

Description of changes:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

codecov-commenter commented 4 months ago

Codecov Report

Attention: Patch coverage is 46.15385% with 7 lines in your changes are missing coverage. Please review.

Project coverage is 82.56%. Comparing base (ed7bbd6) to head (5a78a56).

Files Patch % Lines
source/v5/mqtt5_client.c 46.15% 7 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #369 +/- ## ========================================== - Coverage 82.62% 82.56% -0.07% ========================================== Files 20 20 Lines 8758 8768 +10 ========================================== + Hits 7236 7239 +3 - Misses 1522 1529 +7 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

sfod commented 3 months ago

This PR relaxes the requirements for mqtt5 client to be valid, for some operations. I'm trying to figure out if this could potentially cause something bad in operations that don't validate client (e.g. https://github.com/awslabs/aws-c-mqtt/blob/v0.10.4/source/v5/mqtt5_client.c#L2109). Maybe I'm being too paranoid.

xiazhvera commented 3 months ago

The PR was to workaround the swift error issue. As there are some concern about adding the error in c-mqtt, I will re-evaluate the solution. Temporary close the pr.