Hi Alexander,

On Mon, Aug 28, 2023 at 12:38:45PM +0000, Stephan, Alexander wrote:
> > I'm fine with this, however I find that the doc is not very clear about
> > what is permitted
> I agree that doc needs some more details. I added the note about the
> iterations and described all the symbolic constants, see at the end of this
> mail.

Thanks.

> > Unless you're having any objection, I'm going to flip type and len below:
> Sorry, that was an oversight on my end. Sure, go ahead. I actually thought I
> would have arranged it that way, but it somehow slipped my view.

No worries, that's what reviews are made for, isn't it ? ;-)

> > In this case it could be clarified in the doc that the sample fetch
> > functions for authority/uniqueid only return the first of each, and that
> > fc_pp_tlv() iterates over all occurrences of the requested ID. This would
> > then place a clear separation between trying to extract THE authority, or
> > checking ALL TLVs of type AUTHORITY.
> 
> > What do you think ?
> Good idea, agreed. That also aligns with my goals to keep the existing
> behavior as much as possible.

OK great.

> > If you're OK with this, I'd appreciate it if you could send a few informal
> > incremental patches that I'd squash into yours.
> I'm OK with this. :-) The first patch (0001) should be squashed into the
> commit that introduces the fetch fc_pp_tlv.
> I guess it fits there because it actually retains and documents prior
> behavior. The behavior regarding duplicates is also already present,
> therefore I already added corresponding docs there.
> The second one  (0002) could then be applied to the last patch with the
> introduction of the constants. Would that work for you?

Overall yes. There are just two small parts in the first patch that are
for the immediately following patches ("refactor...") that I'm going to
move there.

I'm doing that and merging it, or I'll get back to you if I have more
questions. Thank you!

Willy

Reply via email to