git: c4ba4aa54718 - main - libifconfig: Overhaul ifconfig_media_* interfaces

[Ryan Moeller]

On 3/6/21 4:07 AM, Kristof Provost wrote:
>
> On 5 Mar 2021, at 20:44, Mark Johnston wrote:
>
>     On Fri, Mar 05, 2021 at 02:40:29PM -0500, Ryan Moeller wrote:
>
>         On 3/5/21 1:19 PM, Konstantin Belousov wrote:
>
>             On Fri, Mar 05, 2021 at 11:23:56AM +0000, Ryan Moeller wrote:
>
>                 The branch main has been updated by freqlabs:
>
>                 URL:
>                 https://cgit.FreeBSD.org/src/commit/?id=c4ba4aa547184ab401204096cdad9def4ab37964
>                 <https://cgit.FreeBSD.org/src/commit/?id=c4ba4aa547184ab401204096cdad9def4ab37964>
>
>                 commit c4ba4aa547184ab401204096cdad9def4ab37964
>                 Author: Ryan Moeller <freqlabs at FreeBSD.org>
>                 AuthorDate: 2021-03-02 10:29:17 +0000
>                 Commit: Ryan Moeller <freqlabs at FreeBSD.org>
>                 CommitDate: 2021-03-05 09:15:55 +0000
>
>                 libifconfig: Overhaul ifconfig_media_* interfaces
>
>                 Define an ifmedia_t type to use for ifmedia words.
>
>                 Add ifconfig_media_lookup_* functions to lookup
>                 ifmedia words by name.
>
>                 Get media options as an array of option names rather
>                 than formatting it
>                 as a comma-delimited list into a buffer.
>
>                 Sprinkle const on static the static description tables
>                 for peace of
>                 mind.
>
>                 Don't need to zero memory allocated by calloc.
>
>                 Reviewed by: kp
>                 MFC after: 2 weeks
>                 Differential Revision:
>                 https://reviews.freebsd.org/D29029
>                 <https://reviews.freebsd.org/D29029>
>                 ---
>                 lib/libifconfig/Makefile | 2 +-
>                 lib/libifconfig/Symbol.map | 9 +-
>                 lib/libifconfig/libifconfig.h | 69 +++++++-
>                 lib/libifconfig/libifconfig_media.c | 339
>                 ++++++++++++++++++++++++------------
>                 share/examples/libifconfig/status.c | 27 ++-
>                 5 files changed, 324 insertions(+), 122 deletions(-)
>
>                 diff --git a/lib/libifconfig/Makefile
>                 b/lib/libifconfig/Makefile
>                 index 73dad36c1dc5..c6f006018427 100644
>                 --- a/lib/libifconfig/Makefile
>                 +++ b/lib/libifconfig/Makefile
>                 @@ -7,7 +7,7 @@ INTERNALLIB= true
>                 LIBADD= m
>
>                 SHLIBDIR?= /lib
>                 -SHLIB_MAJOR= 1
>                 +SHLIB_MAJOR= 2
>
>                 VERSION_DEF= ${LIBCSRCDIR}/Versions.def
>                 SYMBOL_MAPS= ${.CURDIR}/Symbol.map
>
>             libifconfig is marked as internal, but we provide symbol
>             versioning for it,
>             and do it in the normal FreeBSD namespace. On one hand,
>             our policy is to
>             not bump symvered libs and to provide binary compat shims
>             as needed, on the
>             other, this is internal lib.
>
>             What is the purpose of maintaining symbol versions for it?
>
>         I have work in progress that changes libifconfig to a private
>         lib. I'm
>         not sure why markj@ added the symbol map, but I've been forced to
>         maintain it for my later changes to work.
>
>     I added it because there was some discussion of making it a public
>     library, and adding a symbol map was a step towards that. If it is
>     only
>     going to be a private library, then there's indeed no reason to
>     maintain
>     it.
>
> Historically it’s been the aspiration (passive voice, because while 
> these have been my hopes I’ve not done any of the work) to make this a 
> public library, so e.g. appliance vendors could have a better way to 
> configure the system than doing |system(“ifconfig em0 10.0.0.1/24”);|.
> Libifconfig is making progress thanks to Ryan’s work, but it’s not yet 
> ready for that sort of use.
>
> Given that, it makes sense to keep it as an internal library for now, 
> and breaking the API is fine. I still hope that we’ll get it to the 
> point where it’ll be ready for public consumption (with stable API).
>
> So I think the symbol map is mostly aspirational right now. If it’s a 
> maintenance burden I guess we can remove it, and re-add it when the 
> time comes.
>
> Best regards,
> Kristof
>

Speaking for myself, I don't find it too much of a burden to keep on top 
of Symbol.map, but it does feel awkward bumping SHLIB_MAJOR if I don't 
have to. If it's not necessary to do so for a private library then I 
will gladly keep it at 2 or even drop it back to 1 if that is preferred 
while I'm still fleshing out the API. I do have the library switched 
from internal to private in my local branch so it can be shared with the 
libifconfig bindings for flua that I've been keeping in sync with the 
library.