| Message ID | 20240620085906.153885-3-whitecrowbar@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | aaron conole |
| Headers | show |
| Series | [ovs-dev,1/4] .gitignore: add clangd configuration file | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | warning | apply and check: warning |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
Grigorii Nazarov <whitecrowbar@gmail.com> writes: > Signed-off-by: Grigorii Nazarov <whitecrowbar@gmail.com> > --- There's only one user left in tree. Maybe it makes sense to remove this? Or at least mark it deprecated or something? It is used quite a bit in OVN project. It doesn't seem like it is worth the overhead of the extra function call in OVS, though. At the very least, maybe it can become an inline function in the json.h file. > lib/json.c | 11 ++++------- > 1 file changed, 4 insertions(+), 7 deletions(-) > > diff --git a/lib/json.c b/lib/json.c > index 001f6e6ab..d40e93857 100644 > --- a/lib/json.c > +++ b/lib/json.c > @@ -127,7 +127,9 @@ static void json_parser_input(struct json_parser *, struct json_token *); > > static void json_error(struct json_parser *p, const char *format, ...) > OVS_PRINTF_FORMAT(2, 3); > - > + > +static void json_serialize_string(const char *, struct ds *); > + > const char * > json_type_to_string(enum json_type type) > { > @@ -987,11 +989,7 @@ exit: > void > json_string_escape(const char *in, struct ds *out) > { > - struct json json = { > - .type = JSON_STRING, > - .string = CONST_CAST(char *, in), > - }; > - json_to_ds(&json, 0, out); > + json_serialize_string(in, out); > } > > static void > @@ -1572,7 +1570,6 @@ static void json_serialize_object(const struct shash *object, > struct json_serializer *); > static void json_serialize_array(const struct json_array *, > struct json_serializer *); > -static void json_serialize_string(const char *, struct ds *); > > /* Converts 'json' to a string in JSON format, encoded in UTF-8, and returns > * that string. The caller is responsible for freeing the returned string,
On Thursday, June 20, 2024 9:58:41 PM GMT+3 Aaron Conole wrote: > There's only one user left in tree. Maybe it makes sense to remove > this? Or at least mark it deprecated or something? It is used quite a > bit in OVN project. It doesn't seem like it is worth the overhead of > the extra function call in OVS, though. At the very least, maybe it can > become an inline function in the json.h file. > Refactoring wasn't primer focus of my work, only optimization. Fourth patch in the series adds one more usage. From the perspective of code organization, exposing json_serialize_string might be undesired. From the perspective of further optimization, the very serialization process is much more expensive, than that call. Moreover, there are other places that are way more expensive, UUID serialization is one example. I considered this to be a low hanging improvement that I can include. All that said, I will happily do some small refactoring regarding this function as to make code the clearer. I'm just not sure myself which way is preferable here. Should I make it inline in header or replace with json_serialize_string? > > lib/json.c | 11 ++++------- > > 1 file changed, 4 insertions(+), 7 deletions(-) > > > > diff --git a/lib/json.c b/lib/json.c > > index 001f6e6ab..d40e93857 100644 > > --- a/lib/json.c > > +++ b/lib/json.c > > @@ -127,7 +127,9 @@ static void json_parser_input(struct json_parser *, > > struct json_token *);> > > static void json_error(struct json_parser *p, const char *format, ...) > > > > OVS_PRINTF_FORMAT(2, 3); > > > > - > > + > > +static void json_serialize_string(const char *, struct ds *); > > + > > > > const char * > > json_type_to_string(enum json_type type) > > { > > > > @@ -987,11 +989,7 @@ exit: > > void > > json_string_escape(const char *in, struct ds *out) > > { > > > > - struct json json = { > > - .type = JSON_STRING, > > - .string = CONST_CAST(char *, in), > > - }; > > - json_to_ds(&json, 0, out); > > + json_serialize_string(in, out); > > > > } > > > > static void > > > > @@ -1572,7 +1570,6 @@ static void json_serialize_object(const struct shash > > *object,> > > struct json_serializer *); > > > > static void json_serialize_array(const struct json_array *, > > > > struct json_serializer *); > > > > -static void json_serialize_string(const char *, struct ds *); > > > > /* Converts 'json' to a string in JSON format, encoded in UTF-8, and > > returns> > > * that string. The caller is responsible for freeing the returned > > string,
diff --git a/lib/json.c b/lib/json.c index 001f6e6ab..d40e93857 100644 --- a/lib/json.c +++ b/lib/json.c @@ -127,7 +127,9 @@ static void json_parser_input(struct json_parser *, struct json_token *); static void json_error(struct json_parser *p, const char *format, ...) OVS_PRINTF_FORMAT(2, 3); - + +static void json_serialize_string(const char *, struct ds *); + const char * json_type_to_string(enum json_type type) { @@ -987,11 +989,7 @@ exit: void json_string_escape(const char *in, struct ds *out) { - struct json json = { - .type = JSON_STRING, - .string = CONST_CAST(char *, in), - }; - json_to_ds(&json, 0, out); + json_serialize_string(in, out); } static void @@ -1572,7 +1570,6 @@ static void json_serialize_object(const struct shash *object, struct json_serializer *); static void json_serialize_array(const struct json_array *, struct json_serializer *); -static void json_serialize_string(const char *, struct ds *); /* Converts 'json' to a string in JSON format, encoded in UTF-8, and returns * that string. The caller is responsible for freeing the returned string,
Signed-off-by: Grigorii Nazarov <whitecrowbar@gmail.com> --- lib/json.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)