smbstatus json/xml output with libxo
Philipp Gesang
philipp.gesang at intra2net.com
Thu Sep 5 10:45:14 UTC 2019
Hi Andrew,
I’m not a team member but I like this patch! I’ve given it a
quick test run on our 4.7-series setup and it does its job.
Some remarks about the patch inline below.
-<| Quoting Andrew Walker via samba-technical <awalker at ixsystems.com>, on Thursday, 2019-08-29 01:36:59 PM |>-
> On Wed, Apr 17, 2019 at 10:55 AM Stefan Metzmacher <metze at samba.org> wrote:
>
> >
> > I think using another library should be avoided, do you really need xml
> > or would you be able to work based on libjansson and only json?
> >
> > metze
> >
> >
> Sorry, it took a while to get around to this. I re-worked it to use
> libjansson per your suggestion.
>
> diff --git a/source3/utils/status.c b/source3/utils/status.c
> index b5d0b5e..84646ce 100644
> --- a/source3/utils/status.c
> +++ b/source3/utils/status.c
> @@ -51,6 +51,12 @@
> #include "cmdline_contexts.h"
> #include "locking/leases_db.h"
>
> +#ifdef HAVE_JANSSON
> +#include <jansson.h>
> +#include "audit_logging.h" /* various JSON helpers */
> +#include "auth/common_auth.h"
> +#endif /* [HAVE_JANSSON] */
> +
> #define SMB_MAXPIDS 2048
> static uid_t Ucrit_uid = 0; /* added by OH */
> static struct server_id Ucrit_pid[SMB_MAXPIDS]; /* Ugly !!! */ /* added by OH */
> @@ -64,9 +70,13 @@ static bool processes_only;
> static bool show_brl;
> static bool numeric_only;
> static bool do_checks = true;
> +static bool json_output = false;
> +static bool resolve_uids = false;
>
> const char *username = NULL;
>
> +static const char *session_dialect_str(uint16_t dialect);
> +
> /* added by OH */
> static void Ucrit_addUid(uid_t uid)
> {
> @@ -118,12 +128,646 @@ static bool Ucrit_addPid( struct server_id pid )
> return True;
> }
>
> +#ifdef HAVE_JANSSON
> +static int print_share_mode_json(struct file_id fid,
> + const struct share_mode_data *d,
> + const struct share_mode_entry *e,
> + void *private_data)
> +{
> + struct json_object jsobj = *(struct json_object *)private_data;
> + struct json_object jsobjint = json_new_object();
> + static int count;
> + static int ret = 0;
> + char *denymode = NULL;
^^^^
const char *, lest GCC yells at you.
> +
> + if (do_checks && !is_valid_share_mode_entry(e)) {
> + return 0;
> + }
> +
> + count++;
> +
> + if (do_checks && !serverid_exists(&e->pid)) {
> + /* the process for this entry does not exist any more */
> + return 0;
> + }
> + if (Ucrit_checkPid(e->pid)) {
> + struct server_id_buf tmp;
> + if (json_is_invalid(&jsobjint)) {
> + return -1;
> + }
> + if (json_add_string(&jsobjint, "pid",
> + server_id_str_buf(e->pid, &tmp)) < 0) {
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
In Samba, function calls in argument lists are frowned upon:
https://git.samba.org/samba.git/?p=samba.git;a=blob;f=README.Coding;h=ac9bcd43065fea7246ecddafaeac389319087628;hb=HEAD#l385
> + goto failure;
^^
Trailing <TAB> character. This is repeated many times below.
> + }
> + if (resolve_uids && (json_add_string(&jsobjint, "username",
> + uidtoname(e->uid)) < 0)) {
^^^^^^^^^^^^^^^^^
Function call in arglist.
> + goto failure;
> + }
> + if (json_add_int(&jsobjint, "uid", (unsigned int)e->uid) < 0) {
^^^^^^^^^^^^^^
.uid here is already unsigned (uint32_t). (int) might be more
appropriate.
> + goto failure;
> + }
> + switch (map_share_mode_to_deny_mode(e->share_access,
> + e->private_options)) {
> + case DENY_NONE: denymode = "DENY_NONE"; break;
> + case DENY_ALL: denymode = "DENY_ALL"; break;
> + case DENY_DOS: denymode = "DENY_DOS"; break;
> + case DENY_READ: denymode = "DENY_READ"; break;
> + case DENY_WRITE:denymode = "DENY_WRITE"; break;
> + case DENY_FCB: denymode = "DENY_FCB"; break;
> + default: {
> + denymode = "UNKNOWN";
> + break;
> + }
> + }
> + struct json_object amask = json_new_object();
^^^^^^^^
s/ /<TAB>/
> + if (json_is_invalid(&amask)) {
> + goto failure;
> + }
> + char *access_mask = NULL;
> + access_mask = talloc_asprintf(NULL, "0x%08x", (unsigned int)e->access_mask);
^^^^^^^^^^^^^^
See e->uid above.
> + if (json_add_string(&amask, "hex", access_mask) < 0) {
> + TALLOC_FREE(access_mask);
> + ret = -1;
> + goto failure;
> + }
> + TALLOC_FREE(access_mask);
> + if (verbose) {
> + if (json_add_bool(&amask, "READ_DATA",
> + (e->access_mask & FILE_READ_DATA)?true:false) < 0) {
> + json_free(&amask);
> + goto failure;
> + }
> + if (json_add_bool(&amask, "WRITE_DATA",
> + (e->access_mask & FILE_WRITE_DATA)?true:false) < 0) {
> + json_free(&amask);
> + goto failure;
> + }
> + if (json_add_bool(&amask, "APPEND_DATA",
> + (e->access_mask & FILE_APPEND_DATA)?true:false) < 0) {
> + json_free(&amask);
> + goto failure;
> + }
> + if (json_add_bool(&amask, "READ_EA",
> + (e->access_mask & FILE_READ_EA)?true:false) < 0) {
> + json_free(&amask);
> + goto failure;
> + }
> + if (json_add_bool(&amask, "WRITE_EA",
> + (e->access_mask & FILE_WRITE_EA)?true:false) < 0) {
> + json_free(&amask);
> + goto failure;
> + }
> + if (json_add_bool(&amask, "EXECUTE",
> + (e->access_mask & FILE_EXECUTE)?true:false) < 0) {
> + json_free(&amask);
> + goto failure;
> + }
> + if (json_add_bool(&amask, "READ_ATTRIBUTES",
> + (e->access_mask & FILE_READ_ATTRIBUTES)?true:false) < 0) {
> + json_free(&amask);
> + goto failure;
> + }
> + if (json_add_bool(&amask, "WRITE_ATTRIBUTES",
> + (e->access_mask & FILE_WRITE_ATTRIBUTES)?true:false) < 0) {
> + json_free(&amask);
> + goto failure;
> + }
> + if (json_add_bool(&amask, "DELETE_CHILD",
> + (e->access_mask & FILE_DELETE_CHILD)?true:false) < 0) {
> + json_free(&amask);
> + goto failure;
> + }
> + if (json_add_bool(&amask, "DELETE",
> + (e->access_mask & SEC_STD_DELETE)?true:false) < 0) {
> + json_free(&amask);
> + goto failure;
> + }
> + if (json_add_bool(&amask, "READ_CONTROL",
> + (e->access_mask & SEC_STD_READ_CONTROL)?true:false) < 0) {
> + json_free(&amask);
> + goto failure;
> + }
> + if (json_add_bool(&amask, "WRITE_DAC",
> + (e->access_mask & SEC_STD_WRITE_DAC)?true:false) < 0) {
> + json_free(&amask);
> + goto failure;
> + }
> + if (json_add_bool(&amask, "WRITE_OWNER",
> + (e->access_mask & SEC_STD_WRITE_OWNER)?true:false) < 0) {
> + json_free(&amask);
> + goto failure;
> + }
> + if (json_add_bool(&amask, "SYNCHRONIZE",
> + (e->access_mask & SEC_STD_SYNCHRONIZE)?true:false) < 0) {
> + json_free(&amask);
> + goto failure;
> + }
> + }
> + if (json_add_object(&jsobjint, "access_mask", &amask) < 0) {
> + json_free(&amask);
> + goto failure;
> + }
> + struct json_object oplock = json_new_object();
> + if (json_is_invalid(&oplock)) {
> + goto failure;
> + }
> +
^^^^^^^^^^^^^^^^^^^^^
Stray tabs.
> + if (json_add_bool(&oplock, "EXCLUSIVE",
> + (e->op_type & EXCLUSIVE_OPLOCK)?true:false) < 0) {
> + json_free(&oplock);
> + goto failure;
> + }
> + if (json_add_bool(&oplock, "BATCH",
> + (e->op_type & BATCH_OPLOCK)?true:false) < 0) {
> + json_free(&oplock);
> + goto failure;
> + }
> + if (json_add_bool(&oplock, "LEVEL_II",
> + (e->op_type & LEVEL_II_OPLOCK)?true:false) < 0) {
> + json_free(&oplock);
> + goto failure;
> + }
> + if (json_add_bool(&oplock, "LEASE",
> + (e->op_type & LEASE_OPLOCK)?true:false) < 0) {
> + json_free(&oplock);
> + goto failure;
> + }
> + if (json_add_object(&jsobjint, "oplock", &oplock) < 0) {
> + json_free(&oplock);
> + goto failure;
> + }
> + struct json_object lease = json_new_object();
> + if (json_output && (e->op_type & LEASE_OPLOCK)) {
> + struct json_object lease = json_new_object();
This leaks lease above! Also below check should be moved out of
the if block, same for the eventual json_add_object(lease).
> + if (json_is_invalid(&lease)) {
> + goto failure;
> + }
> + NTSTATUS status;
> + uint32_t lstate;
> +
> + status = leases_db_get(
> + &e->client_guid,
> + &e->lease_key,
> + &d->id,
> + &lstate, /* current_state */
> + NULL, /* breaking */
> + NULL, /* breaking_to_requested */
> + NULL, /* breaking_to_required */
> + NULL, /* lease_version */
> + NULL); /* epoch */
> +
> + if (NT_STATUS_IS_OK(status)) {
> + if (json_add_bool(&lease, "READ",
> + (lstate & SMB2_LEASE_READ)?true:false) < 0) {
> + json_free(&lease);
> + goto failure;
> + }
> + if (json_add_bool(&lease, "WRITE",
> + (lstate & SMB2_LEASE_WRITE)?true:false) < 0) {
> + json_free(&lease);
> + goto failure;
> + }
> + if (json_add_bool(&lease, "HANDLE",
> + (lstate & SMB2_LEASE_HANDLE)?true:false) < 0) {
> + json_free(&lease);
> + goto failure;
> + }
> + }
> + if (json_add_object(&jsobjint, "lease", &lease) < 0) {
> + json_free(&lease);
> + goto failure;
> + }
> + }
> + else {
> + if (json_add_bool(&lease, "READ", false) < 0) {
> + json_free(&lease);
> + goto failure;
> + }
> + if (json_add_bool(&lease, "WRITE", false) < 0) {
> + json_free(&lease);
> + goto failure;
> + }
> + if (json_add_bool(&lease, "HANDLE", false) < 0) {
> + json_free(&lease);
> + goto failure;
> + }
> + if (json_add_object(&jsobjint, "lease", &lease) < 0) {
> + json_free(&lease);
> + goto failure;
> + }
> + }
> + if (json_add_string(&jsobjint, "service_path", d->servicepath) < 0) {
> + goto failure;
> + }
> + char *filename = NULL;
> + filename = talloc_asprintf(NULL, "%s%s", d->base_name,
> + (d->stream_name != NULL) ? d->stream_name : "");
> + if (json_add_string(&jsobjint, "filename", filename) < 0) {
> + TALLOC_FREE(filename);
> + goto failure;
> + }
> + TALLOC_FREE(filename);
> + if (json_add_object(&jsobj, NULL, &jsobjint) < 0) {
> + goto failure;
> + }
> + }
> +
> + return 0;
> +
> +failure:
> + json_free(&jsobjint);
> + return -1;
> +}
> +
> +static void print_brl_json(struct file_id id,
> + struct server_id pid,
^
Trailing space.
> + enum brl_type lock_type,
> + enum brl_flavour lock_flav,
> + br_off start,
> + br_off size,
> + void *private_data)
> +{
> + struct json_object jsobj = *(struct json_object *)private_data;
> + struct json_object jsobjint = json_new_object();
> + static int count;
> + unsigned int i;
> + static const struct {
> + enum brl_type lock_type;
> + const char *desc;
> + } lock_types[] = {
> + { READ_LOCK, "R" },
> + { WRITE_LOCK, "W" },
> + { UNLOCK_LOCK, "U" }
> + };
> + const char *desc="X";
> + const char *sharepath = "";
> + char *fname = NULL;
> + struct share_mode_lock *share_mode;
struct share_mode_lock *share_mode = NULL;
> + struct server_id_buf tmp;
> +
> + count++;
> +
> + share_mode = fetch_share_mode_unlocked(NULL, id);
> + if (share_mode) {
> + bool has_stream = share_mode->data->stream_name != NULL;
> +
> + fname = talloc_asprintf(NULL, "%s%s%s",
> + share_mode->data->base_name,
> + has_stream ? ":" : "",
> + has_stream ?
> + share_mode->data->stream_name :
> + "");
> + } else {
> + fname = talloc_strdup(NULL, "");
> + if (fname == NULL) {
> + return;
> + }
> + }
> +
> + for (i=0;i<ARRAY_SIZE(lock_types);i++) {
> + if (lock_type == lock_types[i].lock_type) {
> + desc = lock_types[i].desc;
> + }
> + }
> + if (json_is_invalid(&jsobjint)) {
> + goto failure;
> + }
> + if (json_add_string(&jsobjint, "pid",
> + server_id_str_buf(pid, &tmp)) < 0) {
> + goto failure;
> + }
> + if (json_add_string(&jsobjint, "dev_inode",
> + file_id_string_tos(&id)) < 0) {
> + goto failure;
> + }
> + if (json_add_string(&jsobjint, "read_write", desc) < 0) {
> + goto failure;
> + }
> + if (json_add_int(&jsobjint, "start", (intmax_t)start) < 0) {
^^^^^^^^^^^^^^^
This conversion is lossy. Unfortunately we don’t currently have
JSON helpers for 64 bits values.
There is WIP patchset that among other things adds
json_add_uint64() and similar APIs:
https://gitlab.com/samba-team/devel/samba/tree/phgsng-net-dominfo
The functions are introduced in these two commits:
× https://gitlab.com/samba-team/devel/samba/commit/6ef567d15c3205a1ec5de74d34add4231f91d0d4
× https://gitlab.com/samba-team/devel/samba/commit/2b0982b298747488225d8e6f99794f0acb58d0e6
which IIRC are self-contained but probably overkill for your
purposes.
> + goto failure;
> + }
> + if (json_add_int(&jsobjint, "size", (intmax_t)size) < 0) {
^^^^^^^^^^^^^^^
See above.
> + goto failure;
> + }
> + if (json_add_string(&jsobjint, "share_path", sharepath) < 0) {
> + goto failure;
> + }
> + if (json_add_string(&jsobjint, "file_name", fname) < 0) {
> + goto failure;
> + }
> + if (json_add_object(&jsobj, NULL, &jsobjint) < 0) {
> + goto failure;
> + }
> +
> + TALLOC_FREE(fname);
> + TALLOC_FREE(share_mode);
> +
> +failure:
> + TALLOC_FREE(fname);
> + TALLOC_FREE(share_mode);
> + json_free(&jsobjint);
> +}
> +
> +static int traverse_connections_json(const struct connections_key *key,
> + const struct connections_data *crec,
> + void *private_data)
> +{
> + struct json_object jsobj = *(struct json_object *)private_data;
> + struct json_object jsobjint = json_new_object();
> + struct server_id_buf tmp;
> + char *timestr = NULL;
> + int result = 0;
> + const char *encryption = "-";
> + const char *signing = "-";
> +
> + if (crec->cnum == TID_FIELD_INVALID)
> + return 0;
> +
> + if (do_checks &&
> + (!process_exists(crec->pid) || !Ucrit_checkUid(crec->uid))) {
> + return 0;
> + }
> +
> + timestr = timestring(NULL, crec->start);
> + if (timestr == NULL) {
> + return -1;
> + }
> +
> + if (smbXsrv_is_encrypted(crec->encryption_flags)) {
> + switch (crec->cipher) {
> + case SMB_ENCRYPTION_GSSAPI:
> + encryption = "GSSAPI";
> + break;
> + case SMB2_ENCRYPTION_AES128_CCM:
> + encryption = "AES-128-CCM";
> + break;
> + case SMB2_ENCRYPTION_AES128_GCM:
> + encryption = "AES-128-GCM";
> + break;
> + default:
> + encryption = "???";
> + result = -1;
> + break;
> + }
> + }
> +
> + if (smbXsrv_is_signed(crec->signing_flags)) {
> + if (crec->dialect >= SMB3_DIALECT_REVISION_302) {
> + signing = "AES-128-CMAC";
> + } else if (crec->dialect >= SMB2_DIALECT_REVISION_202) {
> + signing = "HMAC-SHA256";
> + } else {
> + signing = "HMAC-MD5";
> + }
> + }
> +
> + if (json_is_invalid(&jsobjint)) {
> + TALLOC_FREE(timestr);
> + return -1;
> + }
> + if (json_add_string(&jsobjint, "service", crec->servicename) < 0) {
> + goto failure;
> + }
> + if (json_add_string(&jsobjint, "pid",
> + server_id_str_buf(crec->pid, &tmp)) < 0) {
> + goto failure;
> + }
> + if (json_add_string(&jsobjint, "machine", crec->machine) < 0) {
> + goto failure;
> + }
> + if (json_add_string(&jsobjint, "connected_at", timestr) < 0) {
> + goto failure;
> + }
> + if (json_add_string(&jsobjint, "encryption", encryption) < 0) {
> + goto failure;
> + }
> + if (json_add_string(&jsobjint, "signing", signing) < 0) {
> + goto failure;
> + }
> + if (json_add_object(&jsobj, NULL, &jsobjint) < 0) {
> + goto failure;
> + }
> +
> + TALLOC_FREE(timestr);
> + return result;
> +
> +failure:
> + TALLOC_FREE(timestr);
> + json_free(&jsobjint);
> + return -1;
> +}
> +
> +static int traverse_sessionid_json(const char *key, struct sessionid *session,
> + void *private_data)
> +{
> + struct json_object jsobj = *(struct json_object *)private_data;
> + struct json_object jsobjint = json_new_object();
> + struct server_id_buf tmp;
> + int result = 0;
> + const char *encryption = "-";
> + const char *signing = "-";
> +
> + if (do_checks &&
> + (!process_exists(session->pid) ||
> + !Ucrit_checkUid(session->uid))) {
> + return 0;
> + }
> +
> + Ucrit_addPid(session->pid);
> +
> + if (json_output && json_is_invalid(&jsobjint)) {
^^^^^^^^^^^
Why check json_output here? Can’t we just assume that when a
“*_json()” method is run?
> + return -1;
> + }
> +
> + if (numeric_only) {
> + if (json_output) {
> + if (json_add_int(&jsobjint, "uid",
> + (unsigned int)session->uid) < 0) {
> + goto failure;
> + }
> + if (json_add_int(&jsobjint, "gid",
> + (unsigned int)session->gid) < 0) {
> + goto failure;
> + }
> + }
> + } else {
> + if (json_output) {
> + if (json_add_int(&jsobjint, "uid",
> + (unsigned int)session->uid) < 0) {
> + goto failure;
> + }
> + if (json_add_int(&jsobjint, "gid",
> + (unsigned int)session->gid) < 0) {
> + goto failure;
> + }
> + }
The above condition could be simplified by moving the “if
(json_output)” block out and then starting here with “if
(!numeric_only)”.
> + if (session->uid == -1 && session->gid == -1) {
> + /*
> + * The session is not fully authenticated yet.
> + */
> + if (json_add_string(&jsobjint, "username",
> + "(auth in progress)") < 0) {
^
Trailing space.
> + goto failure;
> + }
> + if (json_add_string(&jsobjint, "groupname",
> + "(auth in progress)") < 0) {
> + goto failure;
> + }
> + } else {
> + /*
> + * In theory it should not happen that one of
> + * session->uid and session->gid is valid (ie != -1)
> + * while the other is not (ie = -1), so we a check for
> + * that case that bails out would be reasonable.
> + */
> + const char *uid_name = "-1";
> + const char *gid_name = "-1";
> +
> + if (session->uid != -1) {
> + uid_name = uidtoname(session->uid);
> + if (uid_name == NULL) {
> + return -1;
> + }
> + }
> + if (session->gid != -1) {
> + gid_name = gidtoname(session->gid);
> + if (gid_name == NULL) {
> + return -1;
> + }
> + }
> + if (json_output) {
The {u,g}id_name values are unused unless json_output is set so
you might as well “else if (json_output)” above.
> + if (json_add_string(&jsobjint, "username",
> + uid_name) < 0) {
> + goto failure;
> + }
> + if (json_add_string(&jsobjint, "groupname",
> + gid_name) < 0) {
> + goto failure;
> + }
> + }
> + }
> + }
> +
> + if (smbXsrv_is_encrypted(session->encryption_flags)) {
> + switch (session->cipher) {
> + case SMB2_ENCRYPTION_AES128_CCM:
> + encryption = "AES-128-CCM";
> + break;
> + case SMB2_ENCRYPTION_AES128_GCM:
> + encryption = "AES-128-GCM";
> + break;
> + default:
> + encryption = "???";
^^^^^
Maybe something like “unknown (%hu)” with the cipher number instead?
> + result = -1;
> + break;
> + }
> + } else if (smbXsrv_is_partially_encrypted(session->encryption_flags)) {
> + switch (session->cipher) {
> + case SMB_ENCRYPTION_GSSAPI:
> + encryption = "partial(GSSAPI)";
> + break;
> + case SMB2_ENCRYPTION_AES128_CCM:
> + encryption = "partial(AES-128-CCM)";
> + break;
> + case SMB2_ENCRYPTION_AES128_GCM:
> + encryption = "partial(AES-128-GCM)";
> + break;
> + default:
> + encryption = "???";
See above.
> + result = -1;
> + break;
> + }
> + }
> +
> + if (smbXsrv_is_signed(session->signing_flags)) {
> + if (session->connection_dialect >= SMB3_DIALECT_REVISION_302) {
> + signing = "AES-128-CMAC";
> + } else if (session->connection_dialect >= SMB2_DIALECT_REVISION_202) {
> + signing = "HMAC-SHA256";
> + } else {
> + signing = "HMAC-MD5";
> + }
> + } else if (smbXsrv_is_partially_signed(session->signing_flags)) {
> + if (session->connection_dialect >= SMB3_DIALECT_REVISION_302) {
> + signing = "partial(AES-128-CMAC)";
> + } else if (session->connection_dialect >= SMB2_DIALECT_REVISION_202) {
> + signing = "partial(HMAC-SHA256)";
> + } else {
> + signing = "partial(HMAC-MD5)";
> + }
> + }
> +
> +
> + if (json_output) {
> + if (json_add_string(&jsobjint, "remote_machine",
> + session->remote_machine) < 0) {
> + goto failure;
> + }
> + if (json_add_string(&jsobjint, "hostname", session->hostname) < 0) {
> + goto failure;
> + }
> + if (json_add_string(&jsobjint, "session_dialect",
> + session_dialect_str(session->connection_dialect)) < 0) {
> + goto failure;
> + }
> + if (json_add_string(&jsobjint, "encryption", encryption) < 0) {
> + goto failure;
> + }
> + if (json_add_string(&jsobjint, "signing", signing) < 0) {
> + goto failure;
> + }
> + if (json_add_object(&jsobj, server_id_str_buf(session->pid, &tmp), &jsobjint) < 0) {
> + goto failure;
> + }
> + }
> +
> + return result;
> +
> +failure:
> + json_free(&jsobjint);
> + return -1;
> +}
> +
> +static bool print_notify_rec_json(const char *path, struct server_id server,
> + const struct notify_instance *instance,
> + void *private_data)
> +{
> + struct server_id_buf idbuf;
> + struct json_object jsobj = *(struct json_object *)private_data;
> + struct json_object jsobjint = json_new_object();
> +
> + if (json_is_invalid(&jsobjint)) {
> + return false;
> + }
> + if (json_add_string(&jsobjint, "pid", server_id_str_buf(server, &idbuf)) < 0) {
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Function call in arglist.
> + goto failure;
> + }
> + if (json_add_string(&jsobjint, "path", path) < 0) {
> + goto failure;
> + }
> + if (json_add_int(&jsobjint, "filter", (unsigned)instance->filter) < 0) {
^^^^^^^^^^^^^^^^^^^^^^^^^^
Already unsigned, I’d prefer (int).
> + goto failure;
> + }
> + if (json_add_int(&jsobjint, "subdir_filter", (unsigned)instance->subdir_filter) < 0) {
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
See above.
> + goto failure;
> + }
> + if (json_add_object(&jsobj, NULL, &jsobjint) < 0) {
> + goto failure;
> + }
> +
> + return true;
> +
> +failure:
> + json_free(&jsobjint);
> + return false;
> +}
> +#endif /* [HAVE_JANSSON] */
> +
> static int print_share_mode(struct file_id fid,
> const struct share_mode_data *d,
> const struct share_mode_entry *e,
> void *private_data)
> {
> - bool resolve_uids = *((bool *)private_data);
> static int count;
>
> if (do_checks && !is_valid_share_mode_entry(e)) {
> @@ -144,6 +788,7 @@ static int print_share_mode(struct file_id fid,
>
> if (Ucrit_checkPid(e->pid)) {
> struct server_id_buf tmp;
> + char *denymode = NULL;
This hunk should be dropped, denymode is only used in the JSON
version.
> d_printf("%-11s ", server_id_str_buf(e->pid, &tmp));
> if (resolve_uids) {
> d_printf("%-14s ", uidtoname(e->uid));
> @@ -156,7 +801,7 @@ static int print_share_mode(struct file_id fid,
> case DENY_ALL: d_printf("DENY_ALL "); break;
> case DENY_DOS: d_printf("DENY_DOS "); break;
> case DENY_READ: d_printf("DENY_READ "); break;
> - case DENY_WRITE:printf("DENY_WRITE "); break;
> + case DENY_WRITE:d_printf("DENY_WRITE "); break;
> case DENY_FCB: d_printf("DENY_FCB "); break;
> default: {
> d_printf("unknown-please report ! "
> @@ -508,7 +1153,6 @@ static int traverse_sessionid(const char *key, struct sessionid *session,
> return result;
> }
>
> -
> static bool print_notify_rec(const char *path, struct server_id server,
> const struct notify_instance *instance,
> void *private_data)
> @@ -530,10 +1174,13 @@ int main(int argc, const char *argv[])
> {
> int c;
> int profile_only = 0;
> + #ifdef HAVE_JANSSON
> + char *json_str = NULL;
Unused variable.
> + struct json_object jsobj = json_new_object();
> + #endif /* [HAVE_JANSSON] */
> bool show_processes, show_locks, show_shares;
> bool show_notify = false;
> - bool resolve_uids = false;
> - poptContext pc = NULL;
> + poptContext pc;
What is the motivation for this change?
Cf. https://git.samba.org/samba.git/?p=samba.git;a=blob;f=README.Coding;h=ac9bcd43065fea7246ecddafaeac389319087628;hb=HEAD#l352
> struct poptOption long_options[] = {
> POPT_AUTOHELP
> {
> @@ -625,6 +1272,14 @@ int main(int argc, const char *argv[])
> .descrip = "Numeric uid/gid"
> },
> {
> + .longName = "json",
> + .shortName = 'j',
> + .argInfo = POPT_ARG_NONE,
> + .arg = NULL,
> + .val = 'j',
> + .descrip = "JSON output"
> + },
> + {
> .longName = "fast",
> .shortName = 'f',
> .argInfo = POPT_ARG_NONE,
> @@ -704,6 +1359,9 @@ int main(int argc, const char *argv[])
> case 'n':
> numeric_only = true;
> break;
> + case 'j':
From a user’s perspective, I would suggest to error out here if
!HAVE_JANSSON.
> + json_output = true;
> + break;
> case 'f':
> do_checks = false;
> break;
> @@ -722,7 +1380,7 @@ int main(int argc, const char *argv[])
> if ( username )
> Ucrit_addUid( nametouid(username) );
>
> - if (verbose) {
> + if (verbose && !json_output) {
> d_printf("using configfile = %s\n", get_dyn_CONFIGFILE());
> }
>
> @@ -756,11 +1414,30 @@ int main(int argc, const char *argv[])
> }
>
> if ( show_processes ) {
> - d_printf("\nSamba version %s\n",samba_version_string());
> - d_printf("%-7s %-12s %-12s %-41s %-17s %-20s %-21s\n", "PID", "Username", "Group", "Machine", "Protocol Version", "Encryption", "Signing");
> - d_printf("----------------------------------------------------------------------------------------------------------------------------------------\n");
> + if (!json_output) {
> + d_printf("\nSamba version %s\n",samba_version_string());
> + d_printf("%-7s %-12s %-12s %-41s %-17s %-20s %-21s\n", "PID", "Username", "Group", "Machine", "Protocol Version", "Encryption", "Signing");
> + d_printf("----------------------------------------------------------------------------------------------------------------------------------------\n");
> + sessionid_traverse_read(traverse_sessionid, frame);
> + }
> + #ifdef HAVE_JANSSON
> + else {
> + struct json_object sessions = json_new_object();
> + if (json_is_invalid(&sessions)) {
> + d_printf("Failed to create JSON object [sessions]\n");
> + ret = -1;
> + goto done;
> + }
>
> - sessionid_traverse_read(traverse_sessionid, frame);
> + sessionid_traverse_read(traverse_sessionid_json, &sessions);
> + if (json_add_object(&jsobj, "sessions", &sessions) < 0) {
> + d_printf("Failed to add JSON object [sessions]\n");
> + json_free(&sessions);
> + ret = -1;
> + goto done;
> + }
> + }
> + #endif /* [HAVE_JANSSON] */
>
> if (processes_only) {
> goto done;
> @@ -771,13 +1448,33 @@ int main(int argc, const char *argv[])
> if (brief) {
> goto done;
> }
> +
> + if (!json_output) {
> + d_printf("\n%-12s %-7s %-13s %-32s %-12s %-12s\n", "Service", "pid", "Machine", "Connected at", "Encryption", "Signing");
> + d_printf("---------------------------------------------------------------------------------------------\n");
> + connections_forall_read(traverse_connections, frame);
> + }
> + #ifdef HAVE_JANSSON
> + else {
> + struct json_object shares_array = json_new_array();
> + if (json_is_invalid(&shares_array)) {
> + d_printf("Failed to create JSON array [shares_array]\n");
> + ret = 1;
> + goto done;
> + }
> + connections_forall_read(traverse_connections_json, &shares_array);
> + if (json_add_object(&jsobj, "shares", &shares_array) < 0) {
> + d_printf("Failed to add JSON array [shares_array]\n");
> + json_free(&shares_array);
> + ret = -1;
> + goto done;
> + }
>
> - d_printf("\n%-12s %-7s %-13s %-32s %-12s %-12s\n", "Service", "pid", "Machine", "Connected at", "Encryption", "Signing");
> - d_printf("---------------------------------------------------------------------------------------------\n");
> -
> - connections_forall_read(traverse_connections, frame);
> -
> - d_printf("\n");
> + if (!json_output) {
> + d_printf("\n");
> + }
> + }
> + #endif /* [HAVE_JANSSON] */
>
> if ( shares_only ) {
> goto done;
> @@ -815,8 +1512,27 @@ int main(int argc, const char *argv[])
> ret = 1;
> goto done;
> }
> -
> - result = share_entry_forall(print_share_mode, &resolve_uids);
> + if (!json_output) {
> + result = share_entry_forall(print_share_mode, NULL);
> + }
> + #if HAVE_JANSSON
> + else {
> + struct json_object locks_array = json_new_array();
> + if (json_is_invalid(&locks_array)) {
> + locking_end();
> + d_printf("Failed to create JSON array [locks_array]\n");
> + ret = 1;
> + goto done;
> + }
> + result = share_entry_forall(print_share_mode_json, &locks_array);
> + if (json_add_object(&jsobj, "locked_files", &locks_array) < 0) {
> + locking_end();
> + d_printf("Failed to add JSON array [locks_array]\n");
> + ret = 1;
> + goto done;
> + }
> + }
> + #endif /* [HAVE_JANSSON] */
>
> if (result == 0) {
> d_printf("No locked files\n");
> @@ -824,11 +1540,31 @@ int main(int argc, const char *argv[])
> d_printf("locked file list truncated\n");
> }
>
> - d_printf("\n");
> + if (!json_output) {
> + d_printf("\n");
> + }
>
> - if (show_brl) {
> + if (show_brl && !json_output) {
> brl_forall(print_brl, NULL);
> }
> + #if HAVE_JANSSON
> + else if (show_brl) {
> + struct json_object brl_array = json_new_array();
> + if (json_is_invalid(&brl_array)) {
> + locking_end();
> + d_printf("Failed to create JSON array [brl_array]\n");
> + ret = 1;
> + goto done;
> + }
> + brl_forall(print_brl_json, &brl_array);
> + if (json_add_object(&jsobj, "brl", &brl_array) < 0) {
> + locking_end();
> + d_printf("Failed to add JSON array [brl_array]\n");
> + ret = 1;
> + goto done;
> + }
> + }
> + #endif /* [HAVE_JANSSON] */
>
> locking_end();
> }
> @@ -841,12 +1577,37 @@ int main(int argc, const char *argv[])
> if (n == NULL) {
> goto done;
> }
> - notify_walk(n, print_notify_rec, NULL);
> + if (!json_output) {
> + notify_walk(n, print_notify_rec, NULL);
> + }
> + #if HAVE_JANSSON
> + else {
> + struct json_object notify_array = json_new_array();
> + if (json_is_invalid(¬ify_array)) {
> + locking_end();
> + d_printf("Failed to create JSON array [notify_array]\n");
> + ret = 1;
> + goto done;
> + }
> + notify_walk(n, print_notify_rec_json, ¬ify_array);
> + if (json_add_object(&jsobj, "notify", ¬ify_array) < 0) {
> + locking_end();
> + d_printf("Failed to add JSON array [notify_array]\n");
> + ret = 1;
> + goto done;
> + }
> + }
> + #endif /* [HAVE_JANSSON] */
> TALLOC_FREE(n);
> }
>
> done:
> - poptFreeContext(pc);
This introduces a memleak.
> + #ifdef HAVE_JANSSON
> + if (json_output) {
> + d_printf("%s\n", json_to_string(frame, &jsobj));
Currently smbstatus will write helpful messages to stdout that
aren’t JSON:
# ./bin/smbstatus --json 2>/dev/null
/home/me/tmp/sambadst/var/lib/samba/lock/locking.tdb not initialised
This is normal if an SMB client has never connected to your server.
# echo $?
0
This is unfortunate for a consumer of the output since there is
no straightforward way of telling that it should not expect a
parseable result. IMO it would be be desirable to either move
these messages to stderr or to exit non-zero.
Even worse:
# ./bin/smbstatus --json -L 2>/dev/null
No locked files
{"locked_files": []}
Here JSON and non-JSON are mixed on stdout.
> + }
> + json_free(&jsobj);
> + #endif HAVE_JANSSON
#endif /* HAVE_JANSSON */
> TALLOC_FREE(frame);
> return ret;
> }
Best regards,
Philipp
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20190905/941d4087/signature.sig>
More information about the samba-technical
mailing list