On Fri, Dec 18, 2020 at 05:45:01PM +0100, Claudio Jeker wrote:
> On Fri, Dec 18, 2020 at 01:46:49PM +0100, Theo Buehler wrote:
> > 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.
> >
>
> In most cases the code expects a non-empty string. For example in the
> rsync.c case neither host nor mod are allowed to be NULL or "".
Yes.
> I guess adding an assert(host && mod) would be enough there.
Right.
> I actually prefer the code to blow up since as mentioned
> empty strings are almost always wrong (e.g. empty filenames or hashes).
> Indeed in all those cases a check for NULL should be added in the
> unmarshal function.
Go ahead. It certainly doesn't make things worse or (more) incorrect.
ok tb
>
> --
> :wq Claudio
