Conversation
This commit is a transfer of GitLab branch 88-read-timeouts-in-fsm.
…-messages-in-continuous
…-support-messages-in-continuous
… 6-support-messages-in-continuous
I added the functionality of having an extra data to the service reauth message in a similar fashion as the pico reauth message It was implemented in a way that previous messages still work, so it is not breaking compatibility
This was in a similar way to what already existed on fsmservice
Remove the timeout between reading a ServiceReauth message and sending the PicoReauth message.
…-support-messages-in-continuous
src/messageservicereauth.c
Outdated
| buffer_delete(messageservicereauth->encryptedData); | ||
| } | ||
|
|
||
| buffer_delete(messageservicereauth->extraData); |
There was a problem hiding this comment.
This should be wrapped with a NULL check, and set to NULL after deletion.
if (messageservicereauth->extraData) {
buffer_delete(messageservicereauth->extraData);
messageservicereauth->extraData = NULL;
}
So should all of the other pointers in the messageservicereauth_delete function. This must have been an earlier error on my part.
There was a problem hiding this comment.
extraData is created in messageservicereauth_new so it will always be there, there is no need to check.
Is there a point in setting to NULL if the whole thing will be freed?
There was a problem hiding this comment.
It's defensive, clean and consistent. If changes to other parts of the code mean it gets freed elsewhere, assuming it's done cleanly (i.e. NULL-ed afterwards), then this code will remain valid.
There was a problem hiding this comment.
I fixed that and changed for the other attributes as well.
| buffer_append_lengthprepend(toEncrypt, sequencenumber_get_raw_bytes(messageservicereauth->sequenceNum), SEQUENCE_NUMBER_LENGTH); | ||
|
|
||
| // Extra data | ||
| buffer_append_buffer_lengthprepend(toEncrypt, messageservicereauth->extraData); |
There was a problem hiding this comment.
This needs adding in to jpico too.
| result = false; | ||
| } | ||
|
|
||
| // length | char extraData[length] (optional) |
There was a problem hiding this comment.
Similarly, jpico needs support for this too.
…' into 6-support-messages-in-continuous
|
I've added #11 for these changes to be reflected in the blocking channel versions of the continuous functions. |
llewelld
left a comment
llewelld
left a comment
There was a problem hiding this comment.
This all looks fine. I created a couple of issues for these changes to be copied over to jpico and the blocking version of the continuous cycle.
See mypico/jpico#1 and #11
2 participants
This adds changes to the FsmService and FsmPico state machines in two ways:
It is possible now to set/receive extra data on FsmPico. I don't particularly like the way this is implemented, but I basically copied as it was already done on FsmService.
In my opinion it would make more sense to get the data in callbacks instead of
get_outbound_extra_dataI added a new function on both state machines to force to send a reauthentication message. This way, the extra data can be sent without waiting for the next cycle.
This branch contains all of these (unfortunately they were all essential to implement this), so please merge them first:
#3
#4
#2