From: Sven Eckelmann <sven@narfation.org>
To: b.a.t.m.a.n@lists.open-mesh.org
Cc: Marek Lindner <mareklindner@neomailbox.ch>
Subject: Re: [PATCH] alfred: notify event listener via unix socket
Date: Sun, 01 May 2022 09:54:26 +0200 [thread overview]
Message-ID: <3246469.CvshgyVVUE@sven-l14> (raw)
In-Reply-To: <20220430105647.340588-1-mareklindner@neomailbox.ch>
[-- Attachment #1: Type: text/plain, Size: 3983 bytes --]
On Saturday, 30 April 2022 12:56:47 CEST Marek Lindner wrote:
> The alfred server instance accepts event notification registration
> via the unix socket. These notification sockets only inform
> registered parties of the availibility of an alfred datatype change.
availability
> +int alfred_client_event_monitor(struct globals *globals)
> +{
[...]
> + fprintf(stdout, "Event: type = %d\n", event_notify.type);
event_notify.type is unsigned and not signed.
> diff --git a/main.c b/main.c
> index 68d6efd..98bf64d 100644
> --- a/main.c
> +++ b/main.c
> @@ -39,6 +39,7 @@ static void alfred_usage(void)
> printf(" -I, --change-interface [interface] change to the specified interface(s)\n");
> printf(" -B, --change-bat-iface [interface] change to the specified batman-adv interface\n");
> printf(" -S, --server-status request server status info such as mode & interfaces\n");
> + printf(" -E, --event-monitor monitor alfred data record update eventss\n");
events
> printf("\n");
> printf("server mode options:\n");
> printf(" -i, --interface specify the interface (or comma separated list of interfaces) to listen on\n");
> @@ -164,6 +165,7 @@ static struct globals *alfred_init(int argc, char *argv[])
> {"change-interface", required_argument, NULL, 'I'},
> {"change-bat-iface", required_argument, NULL, 'B'},
> {"server-status", required_argument, NULL, 'S'},
> + {"event-monitor", required_argument, NULL, 'E'},
Why does it require an argument but the usage doesn't describe the argument?
See also getopt_long which also doesn't expect an argument
> @@ -138,10 +140,17 @@ static int unix_sock_add_data(struct globals *globals,
> free(dataset);
> goto err;
> }
> + new_entry_created = true;
> }
> dataset->data_source = SOURCE_LOCAL;
> clock_gettime(CLOCK_MONOTONIC, &dataset->last_seen);
>
> + /* check that data was changed */
> + if (new_entry_created ||
> + dataset->data.header.length != data_len ||
> + memcmp(dataset->buf, data->data, data_len) != 0)
> + unix_sock_event_notify(globals, data->header.type);
> +
I am wondering now if it could be interesting for the listener whether the
data is from us or some remote (for example via the source mac). Does anyone
else have an opinion about that?
> +static int unix_sock_register_listener(struct globals *globals, int client_sock)
> +{
> + struct event_listener *listener;
> + int ret;
> +
> + ret = fcntl(client_sock, F_GETFL, 0);
> + if (ret < 0) {
> + perror("failed to get file status flags");
> + goto err;
> + }
> +
> + ret = fcntl(client_sock, F_SETFL, ret | O_NONBLOCK);
> + if (ret < 0) {
> + perror("failed to set file status flags");
> + goto err;
> + }
> +
> + listener = malloc(sizeof(*listener));
> + if (!listener)
> + goto err;
> +
> + listener->fd = client_sock;
> + INIT_LIST_HEAD(&listener->list);
> + list_add_tail(&listener->list, &globals->event_listeners);
INIT_LIST_HEAD (of the prev/next pointer) is not necessary when you just
overwrite the next/prev pointer in the next line via list_add_tail
> +
> +static void unix_sock_event_listener_free(struct event_listener *listener)
> +{
> + list_del_init(&listener->list);
> + close(listener->fd);
> + free(listener);
list_del_init has no benefit (only downsides) when you free the memory anyway
at the end of the function
> +int unix_sock_events_select_prepare(struct globals *globals, fd_set *fds,
> + fd_set *errfds, int maxsock)
> +{
> + struct event_listener *listener;
> +
> + list_for_each_entry(listener, &globals->event_listeners, list) {
> + if (listener->fd < 0)
> + continue;
> +
> + FD_SET(listener->fd, fds);
> + FD_SET(listener->fd, errfds);
> +
> + if (maxsock < listener->fd)
> + maxsock = listener->fd;
> + }
> +
> + return maxsock;
> +}
I should really rewrite the socket mainloop using epoll.... but this is just a
side note and not a problem in your patch.
Kind regards,
Sven
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2022-05-01 7:54 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-30 10:56 [PATCH] alfred: notify event listener via unix socket Marek Lindner
2022-05-01 7:54 ` Sven Eckelmann [this message]
2022-05-01 9:10 ` Marek Lindner
2022-05-01 9:12 ` Sven Eckelmann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3246469.CvshgyVVUE@sven-l14 \
--to=sven@narfation.org \
--cc=b.a.t.m.a.n@lists.open-mesh.org \
--cc=mareklindner@neomailbox.ch \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.