Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove the use of common buffer #199

Merged
merged 38 commits into from
Aug 29, 2022
Merged

Remove the use of common buffer #199

merged 38 commits into from
Aug 29, 2022

Conversation

AniruddhaKanhere
Copy link
Member

This PR removes the use of common buffer used for creating and transmitting packets. The reception will be handled in the user provided buffer.

paulbartell
paulbartell previously approved these changes Aug 25, 2022
View reviewed changes
paulbartell
paulbartell reviewed Aug 26, 2022
View reviewed changes
source/core_mqtt.c

/*-----------------------------------------------------------*/

static void addWillAndConnectInfo( const MQTTConnectInfo_t * pConnectInfo,
Copy link
Contributor

Choose a reason for hiding this comment

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

addWillAndConnectInfo is only called from sendConnectWithoutCopy. Can we incorporate addWillAndConnectInfo into that function instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

Doing that increases the complexity score.

paulbartell
paulbartell reviewed Aug 26, 2022
View reviewed changes
source/core_mqtt.c
if( pWillInfo != NULL )
{
/* Serialize the topic. */
iterator = addEncodedStringToVector( serializedTopicLength,
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure that this improves clarity really. Wouldn't it be simpler just to fill the array?

paulbartell
paulbartell reviewed Aug 26, 2022
View reviewed changes
source/core_mqtt.c
( long int ) bytesSent ) );
}
/* Send MQTT SUBSCRIBE packet. */
status = sendSubscribeWithoutCopy( pContext,
Copy link
Contributor

Choose a reason for hiding this comment

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

Does sendSubscribeWithoutCopy need to be a separate function?

paulbartell
paulbartell previously approved these changes Aug 29, 2022
View reviewed changes
@AniruddhaKanhere AniruddhaKanhere merged commit 555bcd9 into FreeRTOS:dev Aug 29, 2022
@AniruddhaKanhere AniruddhaKanhere deleted the RmvCmnBuffer branch August 31, 2022 22:26
@AniruddhaKanhere AniruddhaKanhere mentioned this pull request Sep 19, 2022
Merged
AniruddhaKanhere added a commit that referenced this pull request Sep 19, 2022
@AniruddhaKanhere @jasonpcarroll
@alfred2g @archigup
Bring in changes from dev branch to main (#214)
ee13cbd 
* Update ci.yml (#197)

* Update ci.yml

* Add main branch in the CI YAML

* Add cancel callback API (#196)

* Add cancel callback command

* Update the function name

* Fix formatting

* Update memory estimates

* Add State to MQTT_ProcessLoop so that it can be called in a non-blocking manner (#198)

* Add stateful process-loop function

* Add extra checks; fix bugs and add description of functions

* Add index based stateful processloop

* Clean up

* Renamed functions to make them more coherent with their function

* Remove unused function declarations

* Fixed failing CI checks from previous commits except unit-test

* Fixed spell check and updated size-table

* Fix CBMC proofs

* Empty-Commit to trigger CBMC proofs

* Fix loop unwinding values in the Makefile

* Add upper bound on the buffer size of MQTT

* Increase minimum limit on buffer size to >0

* Add upper bound on the size of the buffer as well

* CBMC: Add memmove stub to accelerate coverage

The commit adds a stub for memmove accelerate CBMC coverage
calculation.  Without this stub, coverage for `MQTT_ProcessLoop` and
`MQTT_ReceiveLoop` fails to converge (gets stuck generating the SAT
formula for the memmove in `receiveSingleIteration`).  This stub
checks that src and dst are nonnull pointers and havocs dst.

* Fix formatting

Co-authored-by: Aniruddha Kanhere <[email protected]>
Co-authored-by: Mark R. Tuttle <[email protected]>

* Remove the use of common buffer (#199)

* Make publish use internal buffers

* Fix comment about packet ID

* Try a different approach for ping and disconnect

* Use writev and flush in conjuction with send

* Update the publish method to use vectors

* Add vectored IO to all functions

* Fix formatting

* Reduce complexity score

* Fix spell check and complexity score

* Fix breaking build

* Add doxygen comments

* Fix doxygen part 2

* Doxygen fix part 3

* Fix doxygen part 4

* Fix some checks

* Fix memory tables

* Fix some small errors

* Fix compiler warnings and breaking CI checks from previous commit

* Fix spell check and doxygen

* Fix a couple of CBMC proofs

* Fix ping and publish proofs

* Update the function name

* Fix more CBMC proofs

* Fix MQTT Connect proof

* Add unwinding loops

* Fix last CBMC proof

* Fix formatting

* Update the Subscribe and Unsubscribe functions

* Fix formatting and doxygen checks

* Fix broken CBMC proofs

* Fix memory statistic table

* Revert changes from serializer source

* update comments to clarify write requirements

* Add a note for write function pointer

* Fix spell check

* Update changelog (#202)

* Update MQTT logging so that log levels of the library do not leak (#205)

* Replace publish state arrays with pointers
Added an MQTT_InIt function for QoS > 0 publishes
Fixed functions which were dealing with state arrays

* Fix CI checks and clean up

* Fix CBMC proofs

* Fix sub and unsub CBMC proofs

* Fix remaining proofs

* Fix remaining CI checks

* Fix spell check

* Minor typo fix (#209)

* Update core_mqtt.h (#208)

* Update core_mqtt.h

* Update core_mqtt.h

Co-authored-by: Aniruddha Kanhere <[email protected]>

* Update documentation and Doxygen comments in the source (#206)

* Update comments of the MQTT_InIt function

* Updating documentation of more of functions

* Fix formatting and spell check

* Update core_mqtt.h

* Add hooks to the source code (#200)

* Add mutex hooks

* Clean up of code

* Add doxygen comments and fix spell check

* Fix LogError call

* Fix formatting and memory table

* Fix dereference failure

* Update the hook names

* Fix broken builds

* Update the macros and variables

* Reword the briefs of hooks and uncrustify

* Fir formatting

* Protect get packet ID

* Fix formatting

* Fix Unit tests (#212)

* Fix Unit tests

* Update unit tests after new changes

* Fix more UT

* add dummy calls to the transport

* fix build error

* Remove usused variables

* Remove unsused variables

* Remove usused variables

* Unsued parameter

* Fix ut failure

* Fix uninitialized unit test variables

* Fix ut expectation

* Fix unit-tests

* Fix unit test uninitialized variable

* increase unit test coverage

* increase unit test coverage

* Fix unit test build

* State coverage 100%

* Serializer 100% coverage

* 100% UT coverage

* Fix formatting

* Fix size table

* Address PR comments

Co-authored-by: Aniruddha Kanhere <[email protected]>

* Remove unnecessary ternaries (#211)

Co-authored-by: Aniruddha Kanhere <[email protected]>

* Fix misra deviations  (#213)

* MISRA compliance update 1

* Fix MISRA errors

* Zero MISRA violations; 13 suppressed

* Fix CI checks

* Update MISRA.md

* Remove deviations

* Fix MISRA.md file

* Fix bug - wasn't incrementing vector length properly.

* Remove unwanted files

* Update comment to clarify the control flow of UT

* Remove 'dev' branch from the CI checks

* Update horrid threshold = 10

Co-authored-by: Aniruddha Kanhere <[email protected]>
Co-authored-by: Mark R. Tuttle <[email protected]>
Co-authored-by: jasonpcarroll <[email protected]>
Co-authored-by: alfred gedeon <[email protected]>
Co-authored-by: Archit Gupta <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@archigup archigup archigup approved these changes

@paulbartell paulbartell paulbartell approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@AniruddhaKanhere @paulbartell @archigup