On 27 May 2018 at 00:55, Andreas Jaggi <andreas.jaggi@xxxxxxxxxxxx> wrote:
> Hi Arturo
>
> Thanks for the review, find below the reworked patch.
> Let me know if there are other parts to improve.
>
Thanks Andreas! the patch looks great.
Minor nitpicks below.
> +static int _connect_socket_unix(struct ulogd_pluginstance *pi)
> +{
> + struct json_priv *op = (struct json_priv *) &pi->private;
> + struct sockaddr_un u_addr;
> + int sfd;
> +
> + if (op->sock != -1) {
> + close(op->sock);
> + op->sock = -1;
> + }
^^^
this socket closing could be your new close_socket() function, right?
> + ulogd_log(ULOGD_DEBUG, "connecting to unix:%s\n",
> + file_ce(pi->config_kset).u.string);
> +
> + sfd = socket(AF_UNIX, SOCK_STREAM, 0);
> + if (sfd == -1) {
> + return -1;
> + }
> + u_addr.sun_family = AF_UNIX;
> + strncpy(u_addr.sun_path, file_ce(pi->config_kset).u.string,
> + sizeof(u_addr.sun_path) - 1);
> + if (connect(sfd, (struct sockaddr *) &u_addr, sizeof(struct sockaddr_un)) == -1) {
> + close(sfd);
> + return -1;
> + }
> +
> + op->sock = sfd;
> +
> + return 0;
> +}
> +
> +static int _connect_socket_net(struct ulogd_pluginstance *pi)
> +{
> + struct json_priv *op = (struct json_priv *) &pi->private;
> + struct addrinfo hints;
> + struct addrinfo *result, *rp;
> + int sfd, s;
> +
> + if (op->sock != -1) {
> + close(op->sock);
> + op->sock = -1;
> + }
> +
^^^
same here
> + ulogd_log(ULOGD_DEBUG, "connecting to %s:%s\n",
> + host_ce(pi->config_kset).u.string,
> + port_ce(pi->config_kset).u.string);
> +
> + memset(&hints, 0, sizeof(struct addrinfo));
> + hints.ai_family = AF_UNSPEC;
> + hints.ai_socktype = op->mode == JSON_MODE_UDP ? SOCK_DGRAM : SOCK_STREAM;
> + hints.ai_protocol = 0;
> + hints.ai_flags = 0;
> +
> + s = getaddrinfo(host_ce(pi->config_kset).u.string,
> + port_ce(pi->config_kset).u.string, &hints, &result);
> + if (s != 0) {
> + ulogd_log(ULOGD_ERROR, "getaddrinfo: %s\n", gai_strerror(s));
> + return -1;
> + }
> +
> + for (rp = result; rp != NULL; rp = rp->ai_next) {
> + int on = 1;
> +
> + sfd = socket(rp->ai_family, rp->ai_socktype,
> + rp->ai_protocol);
> + if (sfd == -1)
> + continue;
> +
> + setsockopt(sfd, SOL_SOCKET, SO_REUSEADDR,
> + (char *) &on, sizeof(on));
> +
> + if (connect(sfd, rp->ai_addr, rp->ai_addrlen) != -1)
> + break;
> +
> + close(sfd);
> + }
> +
> + freeaddrinfo(result);
> +
> + if (rp == NULL) {
> + return -1;
> + }
> +
> + op->sock = sfd;
> +
> + return 0;
> +}
> +
> +static int _connect_socket(struct ulogd_pluginstance *pi)
> +{
> + struct json_priv *op = (struct json_priv *) &pi->private;
> +
> + if (op->mode == JSON_MODE_UNIX)
> + return _connect_socket_unix(pi);
> + else
> + return _connect_socket_net(pi);
> +}
> +
> +static int json_interp_socket(struct ulogd_pluginstance *upi, char *buf, int buflen)
> +{
> + struct json_priv *opi = (struct json_priv *) &upi->private;
> + int ret = 0;
> +
> + if (opi->sock != -1)
> + ret = send(opi->sock, buf, buflen, MSG_NOSIGNAL);
> + free(buf);
> + if (ret != buflen) {
> + ulogd_log(ULOGD_ERROR, "Failure sending message: %s\n",
> + strerror(errno));
> + if (ret == -1 || opi->sock == -1)
> + return _connect_socket(upi);
> + else
> + return ULOGD_IRET_ERR;
> + }
> +
> + return ULOGD_IRET_OK;
> +}
> +
> +static int json_interp_file(struct ulogd_pluginstance *upi, char *buf)
> +{
> + struct json_priv *opi = (struct json_priv *) &upi->private;
> +
> + fprintf(opi->of, "%s", buf);
> + free(buf);
> +
> + if (upi->config_kset->ces[JSON_CONF_SYNC].u.value != 0)
> + fflush(opi->of);
> +
> + return ULOGD_IRET_OK;
> +}
> +
> #define MAX_LOCAL_TIME_STRING 38
>
> static int json_interp(struct ulogd_pluginstance *upi)
> {
> struct json_priv *opi = (struct json_priv *) &upi->private;
> unsigned int i;
> + char *buf;
> + int buflen;
> json_t *msg;
>
> msg = json_object();
> @@ -218,34 +389,65 @@ static int json_interp(struct ulogd_pluginstance *upi)
> }
> }
>
> - json_dumpf(msg, opi->of, 0);
> - fprintf(opi->of, "\n");
>
> + buf = json_dumps(msg, 0);
> json_decref(msg);
> + if (buf == NULL) {
> + ulogd_log(ULOGD_ERROR, "Could not create message\n");
> + return ULOGD_IRET_ERR;
> + }
> + buflen = strlen(buf);
> + buf = realloc(buf, sizeof(char)*(buflen+2));
> + if (buf == NULL) {
> + ulogd_log(ULOGD_ERROR, "Could not create message\n");
> + return ULOGD_IRET_ERR;
> + }
> + strncat(buf, "\n", 1);
> + buflen++;
>
> - if (upi->config_kset->ces[JSON_CONF_SYNC].u.value != 0)
> - fflush(opi->of);
> + if (opi->mode == JSON_MODE_FILE)
> + return json_interp_file(upi, buf);
> + else
> + return json_interp_socket(upi, buf, buflen);
> +}
>
> - return ULOGD_IRET_OK;
> +static void reopen_file(struct ulogd_pluginstance *upi)
> +{
> + struct json_priv *oi = (struct json_priv *) &upi->private;
> + FILE *old = oi->of;
> +
> + ulogd_log(ULOGD_NOTICE, "JSON: reopening logfile\n");
> + oi->of = fopen(upi->config_kset->ces[0].u.string, "a");
> + if (!oi->of) {
> + ulogd_log(ULOGD_ERROR, "can't open JSON "
> + "log file: %s\n",
> + strerror(errno));
> + oi->of = old;
> + } else {
> + fclose(old);
> + }
> +}
> +
> +static void reopen_socket(struct ulogd_pluginstance *upi)
> +{
> + ulogd_log(ULOGD_NOTICE, "JSON: reopening socket\n");
> + if (_connect_socket(upi) < 0) {
> + ulogd_log(ULOGD_ERROR, "can't open JSON "
> + "socket: %s\n",
> + strerror(errno));
> + }
> }
>
> static void sighup_handler_print(struct ulogd_pluginstance *upi, int signal)
> {
> struct json_priv *oi = (struct json_priv *) &upi->private;
> - FILE *old = oi->of;
>
> switch (signal) {
> case SIGHUP:
> - ulogd_log(ULOGD_NOTICE, "JSON: reopening logfile\n");
> - oi->of = fopen(upi->config_kset->ces[0].u.string, "a");
> - if (!oi->of) {
> - ulogd_log(ULOGD_ERROR, "can't open JSON "
> - "log file: %s\n",
> - strerror(errno));
> - oi->of = old;
> - } else {
> - fclose(old);
> - }
> + if (oi->mode == JSON_MODE_FILE)
> + reopen_file(upi);
> + else
> + reopen_socket(upi);
> break;
> default:
> break;
> @@ -255,6 +457,8 @@ static void sighup_handler_print(struct ulogd_pluginstance *upi, int signal)
> static int json_configure(struct ulogd_pluginstance *upi,
> struct ulogd_pluginstance_stack *stack)
> {
> + struct json_priv *op = (struct json_priv *) &upi->private;
> + char *mode_str = mode_ce(upi->config_kset).u.string;
> int ret;
>
> ret = ulogd_wildcard_inputkeys(upi);
> @@ -265,13 +469,25 @@ static int json_configure(struct ulogd_pluginstance *upi,
> if (ret < 0)
> return ret;
>
> + if (!strcasecmp(mode_str, "udp")) {
> + op->mode = JSON_MODE_UDP;
> + } else if (!strcasecmp(mode_str, "tcp")) {
> + op->mode = JSON_MODE_TCP;
> + } else if (!strcasecmp(mode_str, "unix")) {
> + op->mode = JSON_MODE_UNIX;
> + } else if (!strcasecmp(mode_str, "file")) {
> + op->mode = JSON_MODE_FILE;
> + } else {
> + ulogd_log(ULOGD_ERROR, "unknown mode '%s'\n", mode_str);
> + return -EINVAL;
> + }
> +
> return 0;
> }
>
> -static int json_init(struct ulogd_pluginstance *upi)
> +static int json_init_file(struct ulogd_pluginstance *upi)
> {
> struct json_priv *op = (struct json_priv *) &upi->private;
> - unsigned int i;
>
> op->of = fopen(upi->config_kset->ces[0].u.string, "a");
> if (!op->of) {
> @@ -280,6 +496,27 @@ static int json_init(struct ulogd_pluginstance *upi)
> return -1;
> }
>
> + return 0;
> +}
> +
> +static int json_init_socket(struct ulogd_pluginstance *upi)
> +{
> + struct json_priv *op = (struct json_priv *) &upi->private;
> +
> + if (host_ce(upi->config_kset).u.string == NULL)
> + return -1;
> + if (port_ce(upi->config_kset).u.string == NULL)
> + return -1;
> +
> + op->sock = -1;
> + return _connect_socket(upi);
> +}
> +
> +static int json_init(struct ulogd_pluginstance *upi)
> +{
> + struct json_priv *op = (struct json_priv *) &upi->private;
> + unsigned int i;
> +
> /* search for time */
> op->sec_idx = -1;
> op->usec_idx = -1;
> @@ -293,15 +530,32 @@ static int json_init(struct ulogd_pluginstance *upi)
>
> *op->cached_tz = '\0';
>
> - return 0;
> + if (op->mode == JSON_MODE_FILE)
> + return json_init_file(upi);
> + else
> + return json_init_socket(upi);
> +}
> +
> +static void close_file(FILE *of) {
> + if (of != stdout)
> + fclose(of);
> +}
> +
> +static void close_socket(struct json_priv *op) {
> + if (op->sock != -1) {
> + close(op->sock);
> + op->sock = -1;
> + }
> }
>
> static int json_fini(struct ulogd_pluginstance *pi)
> {
> struct json_priv *op = (struct json_priv *) &pi->private;
>
> - if (op->of != stdout)
> - fclose(op->of);
> + if (op->mode == JSON_MODE_FILE)
> + close_file(op->of);
> + else
> + close_socket(op);
>
> return 0;
> }
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
