On Fri, Dec 18, 2020 at 11:43:40AM +0100, Claudio Jeker wrote:
> On Wed, Dec 02, 2020 at 05:06:28PM +0100, Claudio Jeker wrote:
> > rpki-client passes both empty strings and NULL strings as zero length
> > objects. The unmarshal code then allocates memory in any case and so a
> > NULL string is unmarshalled as empty string. This is not great, currently
> > there are no empty strings but a fair amount of NULL strings.
> > This diff changes the behaviour and now NULL is passed as NULL.
> > This should simplify passing optional strings (e.g. in the entity queue
> > code).
> 
> I will commit this later today. It will help with some further cleanup of
> the codebase.

I'm a bit torn about this. Some of the callers clearly do not expect
having to deal with NULL.

I see some *printf("%s", NULL) (for example in proc_rsync()) that should
never happen but can now happen with this change. I'm unsure that there
are no NULL derefs that this introduces. I'm fine with this going in if
you intend to address this as part of the further cleanup work you
envision.

> 
> -- 
> :wq Claudio
> 
> ? obj
> Index: io.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/io.c,v
> retrieving revision 1.10
> diff -u -p -r1.10 io.c
> --- io.c      2 Dec 2020 15:31:15 -0000       1.10
> +++ io.c      2 Dec 2020 15:54:38 -0000
> @@ -153,7 +153,7 @@ io_buf_read_alloc(int fd, void **res, si
>  }
>  
>  /*
> - * Read a string (which may just be \0 and zero-length), allocating
> + * Read a string (returns NULL for zero-length strings), allocating
>   * space for it.
>   */
>  void
> @@ -162,6 +162,10 @@ io_str_read(int fd, char **res)
>       size_t   sz;
>  
>       io_simple_read(fd, &sz, sizeof(size_t));
> +     if (sz == 0) {
> +             *res = NULL;
> +             return;
> +     }
>       if ((*res = calloc(sz + 1, 1)) == NULL)
>               err(1, NULL);
>       io_simple_read(fd, *res, sz);
> 

Reply via email to