Re: rpki-client io cleanup
On Thu, Nov 19, 2020 at 12:31:27PM +0100, Claudio Jeker wrote:
> The io marshall code in rpki-client is a bit strange. It mixes
> non-blocking and blocking sematics and some of the code could be more
> async. This is the first mini step. Always use the buffer io API and
> remove the functions that call io_simple_write() internally.
>
> Next step would be to build a proper write queue and kill
> io_simple_write().
>
> Change is fairly mechanical and works for me :)
> --
> :wq Claudio
ok tb
>
> Index: extern.h
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
> retrieving revision 1.34
> diff -u -p -r1.34 extern.h
> --- extern.h 12 Sep 2020 15:46:48 - 1.34
> +++ extern.h 19 Nov 2020 11:17:42 -
> @@ -378,10 +378,8 @@ void io_simple_write(int, const void *
> void io_buf_buffer(char **, size_t *, size_t *, const void *,
> size_t);
> void io_buf_read_alloc(int, void **, size_t *);
> -void io_buf_write(int, const void *, size_t);
> void io_str_buffer(char **, size_t *, size_t *, const char *);
> void io_str_read(int, char **);
> -void io_str_write(int, const char *);
>
> /* X509 helpers. */
>
> Index: io.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/io.c,v
> retrieving revision 1.9
> diff -u -p -r1.9 io.c
> --- io.c 12 Sep 2020 15:46:48 - 1.9
> +++ io.c 19 Nov 2020 11:17:05 -
> @@ -98,17 +98,6 @@ io_buf_buffer(char **b, size_t *bsz,
> }
>
> /*
> - * Write a binary buffer of the given size, which may be zero.
> - */
> -void
> -io_buf_write(int fd, const void *p, size_t sz)
> -{
> -
> - io_simple_write(fd, &sz, sizeof(size_t));
> - io_simple_write(fd, p, sz);
> -}
> -
> -/*
> * Like io_str_write() but into a buffer.
> */
> void
> @@ -117,17 +106,6 @@ io_str_buffer(char **b, size_t *bsz, siz
> size_t sz = (p == NULL) ? 0 : strlen(p);
>
> io_buf_buffer(b, bsz, bmax, p, sz);
> -}
> -
> -/*
> - * Write a NUL-terminated string, which may be zero-length.
> - */
> -void
> -io_str_write(int fd, const char *p)
> -{
> - size_t sz = (p == NULL) ? 0 : strlen(p);
> -
> - io_buf_write(fd, p, sz);
> }
>
> /*
> Index: main.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v
> retrieving revision 1.84
> diff -u -p -r1.84 main.c
> --- main.c24 Oct 2020 08:12:00 - 1.84
> +++ main.c19 Nov 2020 11:20:01 -
> @@ -301,6 +301,8 @@ repo_lookup(int fd, const char *uri)
> const char *host, *mod;
> size_t hostsz, modsz, i;
> struct repo *rp;
> + char*b = NULL;
> + size_t bsz = 0, bmax = 0;
>
> if (!rsync_uri_parse(&host, &hostsz,
> &mod, &modsz, NULL, NULL, NULL, uri))
> @@ -337,9 +339,12 @@ repo_lookup(int fd, const char *uri)
>
> if (!noop) {
> logx("%s/%s: pulling from network", rp->host, rp->module);
> - io_simple_write(fd, &i, sizeof(size_t));
> - io_str_write(fd, rp->host);
> - io_str_write(fd, rp->module);
> + io_simple_buffer(&b, &bsz, &bmax, &i, sizeof(size_t));
> + io_str_buffer(&b, &bsz, &bmax, rp->host);
> + io_str_buffer(&b, &bsz, &bmax, rp->module);
> +
> + io_simple_write(fd, b, bsz);
> + free(b);
> } else {
> rp->loaded = 1;
> logx("%s/%s: using cache", rp->host, rp->module);
> Index: rsync.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/rsync.c,v
> retrieving revision 1.9
> diff -u -p -r1.9 rsync.c
> --- rsync.c 12 Sep 2020 15:46:48 - 1.9
> +++ rsync.c 19 Nov 2020 11:16:17 -
> @@ -171,10 +171,10 @@ proc_child(int signal)
> void
> proc_rsync(char *prog, char *bind_addr, int fd)
> {
> - size_t id, i, idsz = 0;
> + size_t id, i, idsz = 0, bsz = 0, bmax = 0;
> ssize_t ssz;
> char*host = NULL, *mod = NULL, *uri = NULL,
> - *dst = NULL, *path, *save, *cmd;
> + *dst = NULL, *path, *save, *cmd, *b = NULL;
> const char *pp;
> pid_tpid;
> char*args[32];
> @@ -265,8 +265,13 @@ proc_rsync(char *prog, char *bind_addr,
> ok = 0;
> }
>
> - io_simple_write(fd, &ids[i].id, sizeof(size_t));
> - io_simple_write(fd, &ok, sizeof(ok));
> + io_simple_buffer(&b, &bsz, &bmax,
> + &ids[i].id, sizeof(size_t));
> +
rpki-client io cleanup
The io marshall code in rpki-client is a bit strange. It mixes
non-blocking and blocking sematics and some of the code could be more
async. This is the first mini step. Always use the buffer io API and
remove the functions that call io_simple_write() internally.
Next step would be to build a proper write queue and kill
io_simple_write().
Change is fairly mechanical and works for me :)
--
:wq Claudio
Index: extern.h
===
RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
retrieving revision 1.34
diff -u -p -r1.34 extern.h
--- extern.h12 Sep 2020 15:46:48 - 1.34
+++ extern.h19 Nov 2020 11:17:42 -
@@ -378,10 +378,8 @@ voidio_simple_write(int, const void *
voidio_buf_buffer(char **, size_t *, size_t *, const void *,
size_t);
voidio_buf_read_alloc(int, void **, size_t *);
-voidio_buf_write(int, const void *, size_t);
voidio_str_buffer(char **, size_t *, size_t *, const char *);
voidio_str_read(int, char **);
-voidio_str_write(int, const char *);
/* X509 helpers. */
Index: io.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/io.c,v
retrieving revision 1.9
diff -u -p -r1.9 io.c
--- io.c12 Sep 2020 15:46:48 - 1.9
+++ io.c19 Nov 2020 11:17:05 -
@@ -98,17 +98,6 @@ io_buf_buffer(char **b, size_t *bsz,
}
/*
- * Write a binary buffer of the given size, which may be zero.
- */
-void
-io_buf_write(int fd, const void *p, size_t sz)
-{
-
- io_simple_write(fd, &sz, sizeof(size_t));
- io_simple_write(fd, p, sz);
-}
-
-/*
* Like io_str_write() but into a buffer.
*/
void
@@ -117,17 +106,6 @@ io_str_buffer(char **b, size_t *bsz, siz
size_t sz = (p == NULL) ? 0 : strlen(p);
io_buf_buffer(b, bsz, bmax, p, sz);
-}
-
-/*
- * Write a NUL-terminated string, which may be zero-length.
- */
-void
-io_str_write(int fd, const char *p)
-{
- size_t sz = (p == NULL) ? 0 : strlen(p);
-
- io_buf_write(fd, p, sz);
}
/*
Index: main.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v
retrieving revision 1.84
diff -u -p -r1.84 main.c
--- main.c 24 Oct 2020 08:12:00 - 1.84
+++ main.c 19 Nov 2020 11:20:01 -
@@ -301,6 +301,8 @@ repo_lookup(int fd, const char *uri)
const char *host, *mod;
size_t hostsz, modsz, i;
struct repo *rp;
+ char*b = NULL;
+ size_t bsz = 0, bmax = 0;
if (!rsync_uri_parse(&host, &hostsz,
&mod, &modsz, NULL, NULL, NULL, uri))
@@ -337,9 +339,12 @@ repo_lookup(int fd, const char *uri)
if (!noop) {
logx("%s/%s: pulling from network", rp->host, rp->module);
- io_simple_write(fd, &i, sizeof(size_t));
- io_str_write(fd, rp->host);
- io_str_write(fd, rp->module);
+ io_simple_buffer(&b, &bsz, &bmax, &i, sizeof(size_t));
+ io_str_buffer(&b, &bsz, &bmax, rp->host);
+ io_str_buffer(&b, &bsz, &bmax, rp->module);
+
+ io_simple_write(fd, b, bsz);
+ free(b);
} else {
rp->loaded = 1;
logx("%s/%s: using cache", rp->host, rp->module);
Index: rsync.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/rsync.c,v
retrieving revision 1.9
diff -u -p -r1.9 rsync.c
--- rsync.c 12 Sep 2020 15:46:48 - 1.9
+++ rsync.c 19 Nov 2020 11:16:17 -
@@ -171,10 +171,10 @@ proc_child(int signal)
void
proc_rsync(char *prog, char *bind_addr, int fd)
{
- size_t id, i, idsz = 0;
+ size_t id, i, idsz = 0, bsz = 0, bmax = 0;
ssize_t ssz;
char*host = NULL, *mod = NULL, *uri = NULL,
- *dst = NULL, *path, *save, *cmd;
+ *dst = NULL, *path, *save, *cmd, *b = NULL;
const char *pp;
pid_tpid;
char*args[32];
@@ -265,8 +265,13 @@ proc_rsync(char *prog, char *bind_addr,
ok = 0;
}
- io_simple_write(fd, &ids[i].id, sizeof(size_t));
- io_simple_write(fd, &ok, sizeof(ok));
+ io_simple_buffer(&b, &bsz, &bmax,
+ &ids[i].id, sizeof(size_t));
+ io_simple_buffer(&b, &bsz, &bmax,
+ &ok, sizeof(ok));
+ io_simple_write(fd, b, bsz);
+ bsz = 0;
+
