Adding a function fjson_object_array_del_idx #155
Conversation
|
Note: this PR is needed by "JSON parsing extension" (rsyslog/rsyslog#2773). |
arraylist.c
Outdated
| * Deleting the idx-th element in the array_list. | ||
| */ | ||
| void | ||
| array_list_del_idx(struct array_list *arr, int idx) |
There was a problem hiding this comment.
not a hard requirement, but we nowadays prefer const modifier where possible. Helps a lot with static analyzers and protects against accidental var modification. I know it's kind of controversial with C programmers, thus not a hard requirement ;-)
There was a problem hiding this comment.
array_list_del_idx(struct array_list *const arr, const int idx)
There was a problem hiding this comment.
Thank you, @rgerhards ! I've added "const" as you suggested.
arraylist.c
Outdated
| } | ||
| if(arr->array[idx]) arr->free_fn(arr->array[idx]); | ||
| arr->length--; | ||
| for ( ; idx < arr->length; idx++) { |
There was a problem hiding this comment.
Indeed, it should have been... :) Thanks! Fixed.
nhosoi
force-pushed
the
json_object_array_del_idx
branch
from
82ba0eb
to
284a897
Compare
arraylist.c
Outdated
| return; | ||
| } | ||
| if(arr->array[idx]) arr->free_fn(arr->array[idx]); | ||
| memmove(arr->array + idx, arr->array + idx + 1, (--arr->length - idx) * sizeof(void *)); |
There was a problem hiding this comment.
if removing the last element, that is, if idx == (arr->length-1) then arr->array + idx + 1 will point to random memory after the array - will probably cause compiler checker errors - in this case, you do not need the memmove
There was a problem hiding this comment.
also when removing the last element, the expression (--arr->length - idx) * sizeof(void *) will be 0 - it is possible that given a 0 argument memmove is a no-op, but it isn't clear from this implementation https://code.woboq.org/userspace/glibc/string/memmove.c.html that src will never be accessed
There was a problem hiding this comment.
Hmmm, this is one of my test cases and valgrind did not complain an invalid memory access... :p
"mmk3":[{"a0":"v0"},{},{"a1":"v1"},{"a2":"v2"},{},{},{"a3":"v3"},{}]
==>
"mmk3": [ { "a0": "v0" }, { "a1": "v1" }, { "a2": "v2" }, { "a3": "v3" } ]
If idx is the last one, the size (--arr->length - idx) * sizeof(void *) would be 0. In the case, memmove just returns?
nhosoi
force-pushed
the
json_object_array_del_idx
branch
2 times, most recently
from
a2d5364
to
4246d40
Compare
| } | ||
| if(arr->array[idx]) arr->free_fn(arr->array[idx]); | ||
| if (--arr->length > idx) { | ||
| memmove(arr->array + idx, arr->array + idx + 1, (arr->length - idx) * sizeof(void *)); |
… idx)", which deletes the idx-th element in the array type object.
nhosoi
force-pushed
the
json_object_array_del_idx
branch
from
4246d40
to
e32df78
Compare
|
Hi @rgerhards, |
|
I "just" need to keep up, be out of office parts of this week. Please note that I also must craft a release so that we can see it in rsyslog. Prefer to do that not under time-pressure, past has shown things go wrong if I try to hurry releases... Sorry for some delay. |
|
Thank you for your quick response, @rgerhards. I understand your concern and I was just hoping to double-check if there is something I'd better do on my side. Thanks. |
|
@rgerhards when will there be a release of libfastjson which includes this commit? |
|
@richm I need to fiddle with some CI checks, not sure if I make it for 8.37 :-( sry for that |
|
This PR was added to the 0.99.8 milestone which is after the release https://github.com/rsyslog/libfastjson/releases/tag/v0.99.8 - should it be added to the 0.99.9 milestone instead? |
|
ping - when is the planned release of v0.99.9? |
libfastjson - Adding "void fjson_object_array_del_idx(struct fjson_object *jso, int idx)", which deletes the idx-th element in the array type object. upstream PR: rsyslog/libfastjson#155 liblognorm - Adding a parameter skipempty to the json field type. If skipempty is set as follows, empty json objects are dropped from the parsed result. %field_name:json:skipempty% If any parameter other than "skipempty" is given ("bogus" in this example), an error message "invalid flag for JSON parser: bogus" is issued. Add test for the skipempty parameter. upstream PR: rsyslog/liblognorm#305
Adding "void fjson_object_array_del_idx(struct fjson_object *jso, int idx)",
which deletes the idx-th element in the array type object.
This patch is a part of work to support a requirement to remove empty json objects in parsing JSON.
Adding parse_json_ex to grammar/rainerscript.c for supporting compact.
Adding parameters variable, alt_variable and compact to mmjsonparse.