Use Janssson API instead of YAJL #40
Conversation
src/oci-umount.c
Outdated
| @@ -905,7 +896,7 @@ char *getJSONstring(FILE *from, size_t chunksize, char *msg) | |||
| { | |||
| struct stat stat_buf; | |||
| char *err = NULL, *JSONstring = NULL; | |||
| size_t nbytes, bufsize; | |||
| size_t nbytes, bufsize; | |||
There was a problem hiding this comment.
Fix tabs to match current behaviour
There was a problem hiding this comment.
ack, will push new commit with tabbing fixed. Thanks
|
We will need to test this manually, but once the indenting is fixed this LGTM |
src/oci-umount.c
Outdated
| char *configData; | ||
| _cleanup_(yajl_tree_freep) yajl_val config_node = NULL; | ||
| _cleanup_(json_decrefp) json_t *config_node = NULL; | ||
| _cleanup_config_mounts_ struct config_mount_info *config_mounts = NULL; | ||
| unsigned config_mounts_len = 0; | ||
| _cleanup_fclose_ FILE *fp = NULL; | ||
|
|
||
| /* 'bundle' must be specified for the OCI hooks, and from there we read the configuration file */ | ||
| const char *bundle_path[] = { "bundle", (const char *)0 }; |
There was a problem hiding this comment.
Are we using bundle_path anymore?
src/oci-umount.c
Outdated
| _cleanup_config_mounts_ struct config_mount_info *config_mounts = NULL; | ||
| unsigned config_mounts_len = 0; | ||
| _cleanup_fclose_ FILE *fp = NULL; | ||
|
|
||
| /* 'bundle' must be specified for the OCI hooks, and from there we read the configuration file */ | ||
| const char *bundle_path[] = { "bundle", (const char *)0 }; | ||
| yajl_val v_bundle_path = yajl_tree_get(node, bundle_path, yajl_t_string); | ||
| json_t *v_bundle_path = json_object_get(node, "bundle"); | ||
| if (!v_bundle_path) { | ||
| const char *bundle_path[] = { "bundlePath", (const char *)0 }; |
There was a problem hiding this comment.
same here. Is this bundle_path variable being used?
src/oci-umount.c
Outdated
| snprintf(errbuf, BUFLEN, "failed to read config data from %s", config_file_name); | ||
| configData = getJSONstring(fp, (size_t)CHUNKSIZE, errbuf); | ||
| snprintf(err.text, sizeof(err.text), "failed to read config data from %s", config_file_name); | ||
| configData = getJSONstring(fp, (size_t)CHUNKSIZE, err.text); |
There was a problem hiding this comment.
All this is part of generic cleanup and has nothing to do with json conversion? Can we break the patch down in to a patch series and all the changes which are cleanup and not related to json converstion, move them into a separate patch. That way reviewing becomes easier.
src/oci-umount.c
Outdated
| @@ -1065,13 +1056,16 @@ static int parseBundle(const char *id, yajl_val *node_ptr, char **rootfs, struct | |||
|
|
|||
| /* Extract values from the config json */ | |||
| const char *mount_points_path[] = {"mounts", (const char *)0 }; | |||
There was a problem hiding this comment.
mount_point_paths not being used anymore?
|
@lsm5, once you push your new changes I will review again. |
YAJL upstream has been pretty much dead for a while now. I used commit bd1c5c9 in libguestfs source for reference. Signed-off-by: Lokesh Mandvekar <[email protected]> modified: Makefile.am modified: configure.ac modified: src/oci-umount.c
YAJL upstream has been pretty much dead for a while now.
I used commit bd1c5c9 in libguestfs source for reference.
Signed-off-by: Lokesh Mandvekar [email protected]
modified: Makefile.am
modified: configure.ac
modified: src/oci-umount.c
@rhvgoyal @rhatdan docker [run,exec,commit,push] work fine with this. PTAL.