While the parsing is still done by 1K buffers, the results
are no longer filtered during the parsing, but the whole JSON
has to live in memory at once, which was also the case before
the NSS plugin dropped its dependency on libvirt_util.
Also, the new parser might be more forgiving of missing elements.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
---
tools/nss/libvirt_nss_macs.c | 263 +++++++++--------------------------
1 file changed, 68 insertions(+), 195 deletions(-)
diff --git a/tools/nss/libvirt_nss_macs.c b/tools/nss/libvirt_nss_macs.c
index f45d149793..dbc0790945 100644
--- a/tools/nss/libvirt_nss_macs.c
+++ b/tools/nss/libvirt_nss_macs.c
@@ -25,179 +25,78 @@
#include <stdlib.h>
#include <fcntl.h>
-#include <yajl/yajl_gen.h>
-#include <yajl/yajl_parse.h>
+#include <json.h>
#include "libvirt_nss_macs.h"
#include "libvirt_nss.h"
-enum {
- FIND_MACS_STATE_START,
- FIND_MACS_STATE_LIST,
- FIND_MACS_STATE_ENTRY,
- FIND_MACS_STATE_ENTRY_MACS,
-};
-
-typedef struct {
- const char *name;
- char ***macs;
- size_t *nmacs;
- int state;
-
- char *key;
- struct {
- char *name;
- char **macs;
- size_t nmacs;
- } entry;
-} findMACsParser;
-
static int
-findMACsParserString(void *ctx,
- const unsigned char *stringVal,
- size_t stringLen)
+findMACsFromJSON(json_object *jobj,
+ const char *name,
+ char ***macs,
+ size_t *nmacs)
{
- findMACsParser *parser = ctx;
-
- DEBUG("Parse string state=%d '%.*s' (map key '%s')",
- parser->state, (int)stringLen, (const char *)stringVal,
- NULLSTR(parser->key));
- if (!parser->key)
- return 0;
-
- if (parser->state == FIND_MACS_STATE_ENTRY) {
- if (strcmp(parser->key, "domain"))
- return 1;
-
- free(parser->entry.name);
- if (!(parser->entry.name = strndup((char *)stringVal, stringLen)))
- return 0;
- } else if (parser->state == FIND_MACS_STATE_ENTRY_MACS) {
- char **macs;
- if (strcmp(parser->key, "macs"))
- return 1;
+ size_t i, j;
+ int len;
- if (!(macs = realloc(parser->entry.macs,
- sizeof(char *) * (parser->entry.nmacs + 1))))
- return 0;
-
- parser->entry.macs = macs;
- if (!(macs[parser->entry.nmacs++] = strndup((char *)stringVal, stringLen)))
- return 0;
- } else {
- return 0;
+ if (!json_object_is_type(jobj, json_type_array)) {
+ ERROR("parsed JSON does not contain the leases array");
+ return -1;
}
- return 1;
-}
-
-
-static int
-findMACsParserMapKey(void *ctx,
- const unsigned char *stringVal,
- size_t stringLen)
-{
- findMACsParser *parser = ctx;
-
- DEBUG("Parse map key state=%d '%.*s'",
- parser->state, (int)stringLen, (const char *)stringVal);
-
- free(parser->key);
- if (!(parser->key = strndup((char *)stringVal, stringLen)))
- return 0;
-
- return 1;
-}
-
-
-static int
-findMACsParserStartMap(void *ctx)
-{
- findMACsParser *parser = ctx;
- DEBUG("Parse start map state=%d", parser->state);
+ len = json_object_array_length(jobj);
+ DEBUG("Found an array of length: %zu", len);
+ for (i = 0; i < len; i++) {
+ json_object *entry = NULL;
+ json_object *domain = NULL;
+ const char *domainName;
+ char **tmpMacs = NULL;
+ size_t newmacs = 0;
+ json_object *macsArray = NULL;
- if (parser->state != FIND_MACS_STATE_LIST)
- return 0;
+ entry = json_object_array_get_idx(jobj, i);
+ if (!entry)
+ return -1;
- free(parser->key);
- parser->key = NULL;
- parser->state = FIND_MACS_STATE_ENTRY;
+ DEBUG("Processing item %zu", i);
- return 1;
-}
+ domain = json_object_object_get(entry, "domain");
+ if (!domain)
+ return -1;
+ domainName = json_object_get_string(domain);
+ if (!domainName)
+ return -1;
-static int
-findMACsParserEndMap(void *ctx)
-{
- findMACsParser *parser = ctx;
- size_t i;
+ DEBUG("Processing domain %s", domainName);
- DEBUG("Parse end map state=%d", parser->state);
+ if (strcasecmp(domainName, name))
+ continue;
- if (parser->entry.name == NULL)
- return 0;
+ macsArray = json_object_object_get(entry, "macs");
+ if (!macsArray)
+ return -1;
- if (parser->state != FIND_MACS_STATE_ENTRY)
- return 0;
+ newmacs = json_object_array_length(macsArray);
+ DEBUG("Found %zu MAC addresses", newmacs);
- if (!strcasecmp(parser->entry.name, parser->name)) {
- char **macs = realloc(*parser->macs,
- sizeof(char *) * ((*parser->nmacs) + parser->entry.nmacs));
- if (!macs)
+ tmpMacs = realloc(*macs, sizeof(char *) * (*nmacs + newmacs + 1));
+ if (!tmpMacs)
return 0;
- *parser->macs = macs;
- for (i = 0; i < parser->entry.nmacs; i++)
- (*parser->macs)[(*parser->nmacs)++] = parser->entry.macs[i];
- } else {
- for (i = 0; i < parser->entry.nmacs; i++)
- free(parser->entry.macs[i]);
- }
- free(parser->entry.macs);
- parser->entry.macs = NULL;
- parser->entry.nmacs = 0;
-
- parser->state = FIND_MACS_STATE_LIST;
-
- return 1;
-}
-
-
-static int
-findMACsParserStartArray(void *ctx)
-{
- findMACsParser *parser = ctx;
-
- DEBUG("Parse start array state=%d", parser->state);
-
- if (parser->state == FIND_MACS_STATE_START)
- parser->state = FIND_MACS_STATE_LIST;
- else if (parser->state == FIND_MACS_STATE_ENTRY)
- parser->state = FIND_MACS_STATE_ENTRY_MACS;
- else
- return 0;
+ *macs = tmpMacs;
- return 1;
-}
+ for (j = 0; j < newmacs; j++) {
+ json_object *macobj = NULL;
+ DEBUG("j: %zu", j);
-static int
-findMACsParserEndArray(void *ctx)
-{
- findMACsParser *parser = ctx;
-
- DEBUG("Parse end array state=%d", parser->state);
-
- if (parser->state == FIND_MACS_STATE_LIST)
- parser->state = FIND_MACS_STATE_START;
- else if (parser->state == FIND_MACS_STATE_ENTRY_MACS)
- parser->state = FIND_MACS_STATE_ENTRY;
- else
- return 0;
-
- return 1;
+ macobj = json_object_array_get_idx(macsArray, j);
+ (*macs)[(*nmacs)++] = strdup(json_object_get_string(macobj));
+ }
+ }
+ return 0;
}
@@ -209,66 +108,47 @@ findMACs(const char *file,
{
int fd = -1;
int ret = -1;
- const yajl_callbacks parserCallbacks = {
- NULL, /* null */
- NULL, /* bool */
- NULL, /* integer */
- NULL, /* double */
- NULL, /* number */
- findMACsParserString,
- findMACsParserStartMap,
- findMACsParserMapKey,
- findMACsParserEndMap,
- findMACsParserStartArray,
- findMACsParserEndArray,
- };
- findMACsParser parserState = {
- .name = name,
- .macs = macs,
- .nmacs = nmacs,
- };
- yajl_handle parser = NULL;
char line[1024];
- size_t i;
+ json_object *jobj = NULL;
+ json_tokener *tok = NULL;
+ enum json_tokener_error jerr = json_tokener_success;
+ int jsonflags = JSON_TOKENER_STRICT | JSON_TOKENER_VALIDATE_UTF8;
+ ssize_t nreadTotal = 0;
int rv;
+ size_t i;
if ((fd = open(file, O_RDONLY)) < 0) {
ERROR("Cannot open %s", file);
goto cleanup;
}
- parser = yajl_alloc(&parserCallbacks, NULL, &parserState);
- if (!parser) {
- ERROR("Unable to create JSON parser");
- goto cleanup;
- }
+ tok = json_tokener_new();
+ json_tokener_set_flags(tok, jsonflags);
- while (1) {
- rv = read(fd, line, sizeof(line));
+ while (jerr != json_tokener_continue) {
+ rv = read(fd, line, sizeof(line) - 1);
if (rv < 0)
goto cleanup;
if (rv == 0)
break;
+ nreadTotal += rv;
- if (yajl_parse(parser, (const unsigned char *)line, rv) !=
- yajl_status_ok) {
- unsigned char *err = yajl_get_error(parser, 1,
- (const unsigned char*)line, rv);
- ERROR("Parse failed %s", (const char *) err);
- yajl_free_error(parser, err);
- goto cleanup;
- }
+ line[rv] = 0;
+
+ jobj = json_tokener_parse_ex(tok, line, rv);
+ jerr = json_tokener_get_error(tok);
}
- if (yajl_complete_parse(parser) != yajl_status_ok) {
- ERROR("Parse failed %s",
- yajl_get_error(parser, 1, NULL, 0));
+ if (nreadTotal > 0 && jerr != json_tokener_success) {
+ ERROR("Cannot parse %s: %s", file, json_tokener_error_desc(jerr));
goto cleanup;
}
- ret = 0;
+ ret = findMACsFromJSON(jobj, name, macs, nmacs);
cleanup:
+ json_object_put(jobj);
+ json_tokener_free(tok);
if (ret != 0) {
for (i = 0; i < *nmacs; i++) {
char *mac = (*macs)[i];
@@ -278,13 +158,6 @@ findMACs(const char *file,
*macs = NULL;
*nmacs = 0;
}
- if (parser)
- yajl_free(parser);
- for (i = 0; i < parserState.entry.nmacs; i++)
- free(parserState.entry.macs[i]);
- free(parserState.entry.macs);
- free(parserState.entry.name);
- free(parserState.key);
if (fd != -1)
close(fd);
return ret;
--
2.45.2On Wed, Aug 14, 2024 at 23:40:26 +0200, Ján Tomko wrote:
> While the parsing is still done by 1K buffers, the results
> are no longer filtered during the parsing, but the whole JSON
> has to live in memory at once, which was also the case before
> the NSS plugin dropped its dependency on libvirt_util.
>
> Also, the new parser might be more forgiving of missing elements.
>
> Signed-off-by: Ján Tomko <jtomko@redhat.com>
> ---
> tools/nss/libvirt_nss_macs.c | 263 +++++++++--------------------------
> 1 file changed, 68 insertions(+), 195 deletions(-)
>
> diff --git a/tools/nss/libvirt_nss_macs.c b/tools/nss/libvirt_nss_macs.c
> index f45d149793..dbc0790945 100644
> --- a/tools/nss/libvirt_nss_macs.c
> +++ b/tools/nss/libvirt_nss_macs.c
> @@ -25,179 +25,78 @@
> #include <stdlib.h>
> #include <fcntl.h>
>
> -#include <yajl/yajl_gen.h>
> -#include <yajl/yajl_parse.h>
> +#include <json.h>
Same issue with linking as outlined in previous patch.
>
> #include "libvirt_nss_macs.h"
> #include "libvirt_nss.h"
>
> -enum {
> - FIND_MACS_STATE_START,
> - FIND_MACS_STATE_LIST,
> - FIND_MACS_STATE_ENTRY,
> - FIND_MACS_STATE_ENTRY_MACS,
> -};
> -
> -typedef struct {
> - const char *name;
> - char ***macs;
> - size_t *nmacs;
> - int state;
> -
> - char *key;
> - struct {
> - char *name;
> - char **macs;
> - size_t nmacs;
> - } entry;
> -} findMACsParser;
> -
>
> static int
> -findMACsParserString(void *ctx,
> - const unsigned char *stringVal,
> - size_t stringLen)
As before please document this.
> +findMACsFromJSON(json_object *jobj,
> + const char *name,
> + char ***macs,
> + size_t *nmacs)
> {
> - findMACsParser *parser = ctx;
> -
> - DEBUG("Parse string state=%d '%.*s' (map key '%s')",
> - parser->state, (int)stringLen, (const char *)stringVal,
> - NULLSTR(parser->key));
> - if (!parser->key)
> - return 0;
> -
> - if (parser->state == FIND_MACS_STATE_ENTRY) {
> - if (strcmp(parser->key, "domain"))
> - return 1;
> -
> - free(parser->entry.name);
> - if (!(parser->entry.name = strndup((char *)stringVal, stringLen)))
> - return 0;
> - } else if (parser->state == FIND_MACS_STATE_ENTRY_MACS) {
> - char **macs;
> - if (strcmp(parser->key, "macs"))
> - return 1;
> + size_t i, j;
One per line.
> + int len;
>
> - if (!(macs = realloc(parser->entry.macs,
> - sizeof(char *) * (parser->entry.nmacs + 1))))
> - return 0;
> -
> - parser->entry.macs = macs;
> - if (!(macs[parser->entry.nmacs++] = strndup((char *)stringVal, stringLen)))
> - return 0;
> - } else {
> - return 0;
> + if (!json_object_is_type(jobj, json_type_array)) {
> + ERROR("parsed JSON does not contain the leases array");
> + return -1;
> }
> - return 1;
> -}
> -
> -
> -static int
> -findMACsParserMapKey(void *ctx,
> - const unsigned char *stringVal,
> - size_t stringLen)
> -{
> - findMACsParser *parser = ctx;
> -
> - DEBUG("Parse map key state=%d '%.*s'",
> - parser->state, (int)stringLen, (const char *)stringVal);
> -
> - free(parser->key);
> - if (!(parser->key = strndup((char *)stringVal, stringLen)))
> - return 0;
> -
> - return 1;
> -}
> -
> -
> -static int
> -findMACsParserStartMap(void *ctx)
> -{
> - findMACsParser *parser = ctx;
>
> - DEBUG("Parse start map state=%d", parser->state);
> + len = json_object_array_length(jobj);
> + DEBUG("Found an array of length: %zu", len);
> + for (i = 0; i < len; i++) {
> + json_object *entry = NULL;
> + json_object *domain = NULL;
> + const char *domainName;
> + char **tmpMacs = NULL;
> + size_t newmacs = 0;
> + json_object *macsArray = NULL;
>
> - if (parser->state != FIND_MACS_STATE_LIST)
> - return 0;
> + entry = json_object_array_get_idx(jobj, i);
> + if (!entry)
> + return -1;
>
> - free(parser->key);
> - parser->key = NULL;
> - parser->state = FIND_MACS_STATE_ENTRY;
> + DEBUG("Processing item %zu", i);
>
> - return 1;
> -}
> + domain = json_object_object_get(entry, "domain");
> + if (!domain)
> + return -1;
>
> + domainName = json_object_get_string(domain);
> + if (!domainName)
> + return -1;
No error reporting in any of these? If no shouldn't this continue
processing the loop? (also below)
>
> -static int
> -findMACsParserEndMap(void *ctx)
> -{
> - findMACsParser *parser = ctx;
> - size_t i;
> + DEBUG("Processing domain %s", domainName);
>
> - DEBUG("Parse end map state=%d", parser->state);
> + if (strcasecmp(domainName, name))
> + continue;
>
> - if (parser->entry.name == NULL)
> - return 0;
> + macsArray = json_object_object_get(entry, "macs");
> + if (!macsArray)
> + return -1;
>
> - if (parser->state != FIND_MACS_STATE_ENTRY)
> - return 0;
> + newmacs = json_object_array_length(macsArray);
> + DEBUG("Found %zu MAC addresses", newmacs);
>
> - if (!strcasecmp(parser->entry.name, parser->name)) {
> - char **macs = realloc(*parser->macs,
> - sizeof(char *) * ((*parser->nmacs) + parser->entry.nmacs));
> - if (!macs)
> + tmpMacs = realloc(*macs, sizeof(char *) * (*nmacs + newmacs + 1));
> + if (!tmpMacs)
> return 0;
So allocation failure is success?
>
> - *parser->macs = macs;
> - for (i = 0; i < parser->entry.nmacs; i++)
> - (*parser->macs)[(*parser->nmacs)++] = parser->entry.macs[i];
> - } else {
> - for (i = 0; i < parser->entry.nmacs; i++)
> - free(parser->entry.macs[i]);
> - }
> - free(parser->entry.macs);
> - parser->entry.macs = NULL;
> - parser->entry.nmacs = 0;
> -
> - parser->state = FIND_MACS_STATE_LIST;
> -
> - return 1;
> -}
> -
> -
> -static int
> -findMACsParserStartArray(void *ctx)
> -{
> - findMACsParser *parser = ctx;
> -
> - DEBUG("Parse start array state=%d", parser->state);
> -
> - if (parser->state == FIND_MACS_STATE_START)
> - parser->state = FIND_MACS_STATE_LIST;
> - else if (parser->state == FIND_MACS_STATE_ENTRY)
> - parser->state = FIND_MACS_STATE_ENTRY_MACS;
> - else
> - return 0;
> + *macs = tmpMacs;
>
> - return 1;
> -}
> + for (j = 0; j < newmacs; j++) {
> + json_object *macobj = NULL;
>
> + DEBUG("j: %zu", j);
This doesn't sound very useful.
>
> -static int
> -findMACsParserEndArray(void *ctx)
> -{
> - findMACsParser *parser = ctx;
> -
> - DEBUG("Parse end array state=%d", parser->state);
> -
> - if (parser->state == FIND_MACS_STATE_LIST)
> - parser->state = FIND_MACS_STATE_START;
> - else if (parser->state == FIND_MACS_STATE_ENTRY_MACS)
> - parser->state = FIND_MACS_STATE_ENTRY;
> - else
> - return 0;
> -
> - return 1;
> + macobj = json_object_array_get_idx(macsArray, j);
> + (*macs)[(*nmacs)++] = strdup(json_object_get_string(macobj));
No return value check from strdup (since you are checking realloc).
> + }
> + }
> + return 0;
> }
>
>
> @@ -209,66 +108,47 @@ findMACs(const char *file,
> {
> int fd = -1;
> int ret = -1;
> - const yajl_callbacks parserCallbacks = {
> - NULL, /* null */
> - NULL, /* bool */
> - NULL, /* integer */
> - NULL, /* double */
> - NULL, /* number */
> - findMACsParserString,
> - findMACsParserStartMap,
> - findMACsParserMapKey,
> - findMACsParserEndMap,
> - findMACsParserStartArray,
> - findMACsParserEndArray,
> - };
> - findMACsParser parserState = {
> - .name = name,
> - .macs = macs,
> - .nmacs = nmacs,
> - };
> - yajl_handle parser = NULL;
> char line[1024];
> - size_t i;
> + json_object *jobj = NULL;
> + json_tokener *tok = NULL;
> + enum json_tokener_error jerr = json_tokener_success;
> + int jsonflags = JSON_TOKENER_STRICT | JSON_TOKENER_VALIDATE_UTF8;
> + ssize_t nreadTotal = 0;
> int rv;
> + size_t i;
>
> if ((fd = open(file, O_RDONLY)) < 0) {
> ERROR("Cannot open %s", file);
> goto cleanup;
> }
>
> - parser = yajl_alloc(&parserCallbacks, NULL, &parserState);
> - if (!parser) {
> - ERROR("Unable to create JSON parser");
> - goto cleanup;
> - }
> + tok = json_tokener_new();
> + json_tokener_set_flags(tok, jsonflags);
>
> - while (1) {
> - rv = read(fd, line, sizeof(line));
> + while (jerr != json_tokener_continue) {
> + rv = read(fd, line, sizeof(line) - 1);
> if (rv < 0)
> goto cleanup;
> if (rv == 0)
> break;
> + nreadTotal += rv;
>
> - if (yajl_parse(parser, (const unsigned char *)line, rv) !=
> - yajl_status_ok) {
> - unsigned char *err = yajl_get_error(parser, 1,
> - (const unsigned char*)line, rv);
> - ERROR("Parse failed %s", (const char *) err);
> - yajl_free_error(parser, err);
> - goto cleanup;
> - }
> + line[rv] = 0;
> +
> + jobj = json_tokener_parse_ex(tok, line, rv);
> + jerr = json_tokener_get_error(tok);
> }
>
> - if (yajl_complete_parse(parser) != yajl_status_ok) {
> - ERROR("Parse failed %s",
> - yajl_get_error(parser, 1, NULL, 0));
> + if (nreadTotal > 0 && jerr != json_tokener_success) {
> + ERROR("Cannot parse %s: %s", file, json_tokener_error_desc(jerr));
> goto cleanup;
> }
>
> - ret = 0;
> + ret = findMACsFromJSON(jobj, name, macs, nmacs);
Same issues with the parser as in patch before.
© 2016 - 2024 Red Hat, Inc.