KV parsing rework with newline termination in mind #128
Conversation
| self.buff += payload | ||
| lines = self.buff.split('\n') |
There was a problem hiding this comment.
Since we're now explicitly adding '\r' in ARMmbed/mbed-os#2702, is there a place where this character can be handled and stripped in this PR?
Ideally the next release of htrun should be able to handle cases where the '\r' is or is not present, since the tool may be used with older releases of mbed OS.
There was a problem hiding this comment.
It'd probably fine just to do a rstrip on the returned line before inserting it into the kvl list: https://docs.python.org/2/library/string.html#string.rstrip
There was a problem hiding this comment.
Since \n is removed from K,V regex any character outside the K.V pattern does not affect us. I have tested it. It works with mbed-os master and ARMmbed/mbed-os#2702
There was a problem hiding this comment.
Since we extract key and value using the K,V regex we don't need any extra processing like rstrip.
There was a problem hiding this comment.
Ah I see @mazimkhan, thanks for explaining. Totally missed that the first time around 😄
|
Changes seem ok to me besides my comments. The main thing I thing we have to be careful of with this PR is not breaking compatibility with older releases |
(<- apparently this means "ship it", look how hip I am!)
Key,Value parsing is reworked to separate key,Value protocol format and need to limit request response based on
\n. See #104 for context. Also affects #123Additionally the
self.buffwas being appended with received strings without being cleared and without being needed as onlyself.buff_idxonward string was parsed. Looks like a memory leak.