Bugs: |
|
---|
Implement a parser for ircv3 and add unit tests to it.
Review Request #1874 — Created Oct. 1, 2022 and submitted
This change got pretty big so I didn't implement unescapping tags yet. I did
however put the unit tests in for escaped tags, but they are currently #if 0'd
out.The unit tests are based on the msg-split test cases from https://github.com/ircdocs/parser-tests/blob/master/tests/msg-split.yaml
Ran the unit tests.
Summary | ID |
---|---|
53c1b425e7d8acebe2e79bd6586cfb39fb09eb9f |
Description | From | Last Updated |
---|---|---|
Wrong guard macro name. |
QuLogic | |
I might just be missing something about the contract around the tags_string but shouldn't this be something like if(tags_string == … |
hub@cerras.dev | |
If it's never NULL, then you don't need the tag_string != NULL. |
QuLogic | |
local_error was already checked and returned, so this can't happen. |
QuLogic | |
handler |
QuLogic | |
I'm surprised this is not triggering anything in valgrind, as g_strfreev frees both params and the strings in it. But … |
QuLogic | |
Inconsistent indent. |
QuLogic | |
Walk |
QuLogic | |
Remove? |
QuLogic | |
The spacing around the braces on .params lines is a bit inconsistent. |
QuLogic | |
Not a big deal, but these are swapped from the order in the test data. |
QuLogic | |
Similar to the other control character test, this string is probably also wrong, but the compiler isn't warning because \x035 … |
QuLogic | |
Ditto. |
QuLogic | |
This is warning about an invalid hex escape sequence with \x0305, as YAML escapes are limited to 2 digits, but … |
QuLogic | |
Same here. |
QuLogic | |
It looks like you're missing one test above this one. |
QuLogic | |
Missing trailing comma also. |
QuLogic | |
Missing trailing comma. |
QuLogic |
-
-
libpurple/protocols/ircv3/purpleircv3parser.c (Diff revision 1) I might just be missing something about the contract around the
tags_string
but shouldn't this be something likeif(tags_string == NULL || *tags_string == '\0')
Change Summary:
Add a link to the source of the unit tests.
Description: |
|
---|
-
-
-
libpurple/protocols/ircv3/purpleircv3parser.c (Diff revision 1) If it's never
NULL
, then you don't need thetag_string != NULL
. -
libpurple/protocols/ircv3/purpleircv3parser.c (Diff revision 1) local_error
was already checked and returned, so this can't happen. -
-
libpurple/protocols/ircv3/purpleircv3parser.c (Diff revision 1) I'm surprised this is not triggering anything in valgrind, as
g_strfreev
frees bothparams
and the strings in it.But those strings were extracted in
purple_ircv3_parser_parse_tags
frommiddle
with duplication, or is exactlytrailing
, which were both freed above.So this should have been a double-free.
-
-
-
-
libpurple/protocols/ircv3/tests/test_ircv3_parser.c (Diff revision 1) The spacing around the braces on
.params
lines is a bit inconsistent. -
libpurple/protocols/ircv3/tests/test_ircv3_parser.c (Diff revision 1) Not a big deal, but these are swapped from the order in the test data.
-
libpurple/protocols/ircv3/tests/test_ircv3_parser.c (Diff revision 1) This is warning about an invalid hex escape sequence with
\x0305
, as YAML escapes are limited to 2 digits, but C ones aren't.I think the only way to write it is to split the string,
"coolguy!~ag@n\x02et\x03" "05w\x0fork.admin"
-
-
-
-
libpurple/protocols/ircv3/tests/test_ircv3_parser.c (Diff revision 1) Similar to the other control character test, this string is probably also wrong, but the compiler isn't warning because
\x035
is okay. It would also need the splitting to be right. -
-
libpurple/protocols/ircv3/tests/test_ircv3_parser.c (Diff revision 1) It looks like you're missing one test above this one.
-
Change Summary:
address issues
Commits: |
|
|||||||
---|---|---|---|---|---|---|---|---|
Diff: |
Revision 2 (+2326 -4) |