Re: [dpdk-dev] [PATCH v2 1/7] cmdline: make implementation opaque

[Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>]

On Wed, 30 Sep 2020 09:11:41 +0100, Kinsella, Ray wrote:
> On 05/08/2020 12:17, Dmitry Kozlyuk wrote:
> > On Wed, 5 Aug 2020 10:31:31 +0100, Kinsella, Ray wrote:  
> >> On 30/07/2020 22:06, Dmitry Kozlyuk wrote:  
> >>> struct cmdline exposes platform-specific members it contains, most
> >>> notably struct termios that is only available on Unix. Make the
> >>> structure opaque.
> >>>
> >>> Remove tests checking struct cmdline content as meaningless.
> >>>
> >>> Add cmdline_get_rdline() to access history buffer.
> >>> The new function is currently used only in tests.    
> >>
> >> Should it be INTERNAL then? Is it useful outside of the test cases?  
> > 
> > There are already exposed rdline_*() functions that require struct rdline
> > pointer, which is now only accessible via this function for struct cmdline
> > instances. Thus, public API would be broken with INTERNAL for such use cases.
> >   
> 
> Right but that runs a bit contrary to what you said in the commit log.
> 
> "Add cmdline_get_rdline() to access history buffer.
> The new function is currently used only in tests."
> 
> In anycase, given the elapse in time since my feedback.
> I will ACK the changes to MAP file. 

I rather meant logical breakage that technical one: no functions are removed,
yes, but if existing applications use cmdline::rdline, they won't have a
legitimate way of getting it anymore. The comment you're quoting was before
the workaround, but the WA exists only to keep ABI and we'd prefer the users
to remove direct accesses of the structure fields by 21.11. To do that, we
give them cmdline_get_rdline() and make it public, not internal.