ok.
> On 29 Jul 2020, at 11:38, Klemens Nanni <[email protected]> wrote:
>
> On Tue, Jul 28, 2020 at 07:09:17PM +0200, Klemens Nanni wrote:
>> bridge_status() and switch_status() do the regular sanity check with
>> SIOCGIFFLAGS, but both functions also call is_switch(), bridge_status()
>> also calls is_bridge().
>>
>> Those is_*() helpers do the same SIOCGIFFLAGS sanity check, making those
>> in *_status() entirely redundant, so I'd like to remove them.
> Small correction: is_bridge() does SIOCGIFFLAGS, is_switch() does not.
>
> Below is a new diff that removes the SIOCGIFFLAGS check form is_bridge()
> as well, leaving the two is_*() helpers to their driver specific ioctls
> alone.
>
>> I'm here since the tpmr(4) ioctl interface transition from trunk to
>> bridge semantics is now complete, so ifconfig(8) now requires tpmr bits
>> to show its members in bridge fashion.
>>
>> One way would be duplicate code into is_tpmr() and tpmr_status() which
>> I've already done, but another approach is to unify all bridge like
>> interfaces under bridge_status().
> With this in, merging switch_status() into bridge_status() is a trivial
> diff, adding tpmr awareness to the mix would then be another diff after
> that.
>
>> Either ways, diff below cleans up and makes for simpler code.
> So this effectively just removes SIOCGIFFLAGS in brconfig.c which are of
> no use, imho. ifconfig.c:getinfo() already checks interfaces flags,
> even more than once, for all interfaces.
>
>> Feedback? OK?
>
>
> Index: brconfig.c
> ===================================================================
> RCS file: /cvs/src/sbin/ifconfig/brconfig.c,v
> retrieving revision 1.25
> diff -u -p -r1.25 brconfig.c
> --- brconfig.c 22 Jan 2020 06:24:07 -0000 1.25
> +++ brconfig.c 29 Jul 2020 00:58:40 -0000
> @@ -762,14 +762,8 @@ bridge_holdcnt(const char *value, int d)
> int
> is_bridge()
> {
> - struct ifreq ifr;
> struct ifbaconf ifbac;
>
> - strlcpy(ifr.ifr_name, ifname, sizeof(ifr.ifr_name));
> -
> - if (ioctl(sock, SIOCGIFFLAGS, (caddr_t)&ifr) == -1)
> - return (0);
> -
> ifbac.ifbac_len = 0;
> strlcpy(ifbac.ifbac_name, ifname, sizeof(ifbac.ifbac_name));
> if (ioctl(sock, SIOCBRDGRTS, (caddr_t)&ifbac) == -1) {
> @@ -783,16 +777,11 @@ is_bridge()
> void
> bridge_status(void)
> {
> - struct ifreq ifr;
> struct ifbrparam bp1, bp2;
>
> if (!is_bridge() || is_switch())
> return;
>
> - strlcpy(ifr.ifr_name, ifname, sizeof(ifr.ifr_name));
> - if (ioctl(sock, SIOCGIFFLAGS, (caddr_t)&ifr) == -1)
> - return;
> -
> bridge_cfg("\t");
>
> bridge_list("\t");
> @@ -1184,13 +1173,7 @@ switch_cfg(char *delim)
> void
> switch_status(void)
> {
> - struct ifreq ifr;
> -
> if (!is_switch())
> - return;
> -
> - strlcpy(ifr.ifr_name, ifname, sizeof(ifr.ifr_name));
> - if (ioctl(sock, SIOCGIFFLAGS, (caddr_t)&ifr) == -1)
> return;
>
> switch_cfg("\t");
