On Tue, Mar 7, 2023 at 3:14 PM HAGIO KAZUHITO(萩尾 一仁) <k-hagio...@nec.com>
wrote:
> On 2023/03/07 13:46, lijiang wrote:
>
> >> > + char str[INET6_ADDRSTRLEN + 1] = {0};
> >> > + char buffer[INET6_ADDRSTRLEN + 4] = {0};
> >>
> >> What are the +1 and +4 for?
> >>
> >>
> >> I noticed that the size of INET6_ADDRSTRLEN is 48 in kernel code as
> below:
> >>
> >> --- a/include/linux/inet.h
> >> +++ b/include/linux/inet.h
> >> +/*
> >> + * These mimic similar macros defined in user-space for inet_ntop(3).
> >> + * See /usr/include/netinet/in.h .
> >> + */
> >> +#define INET_ADDRSTRLEN (16)
> >> +#define INET6_ADDRSTRLEN (48)
> >>
> >>
> >> And, the size of INET6_ADDRSTRLEN is 46 in my machine as below:
> >>
> >> # cat /usr/include/netinet/in.h |grep INET6_ADDRSTRLEN
> >> #define INET6_ADDRSTRLEN 46
>
> I'm not sure why the kernel one is 48 (for a multiple of 8?), but
> the INET6_ADDRSTRLEN in glibc header should mean the longest result
> of inet_ntop() in glibc, and the latest glibc also has the same 46.
>
> So I don't think we need to consider the kernel one.
>
> >>
> >>
> >> The rfc2460 said that the IPv6 increases the IP address size from 32
> bits to 128 bits.
> >>
> >> crash> struct in6_addr
> >> struct in6_addr {
> >> union {
> >> __u8 u6_addr8[16];
> >> __be16 u6_addr16[8];
> >> __be32 u6_addr32[4];
> >> } in6_u;
> >> }
> >> SIZE: 16
> >>
> >> Given that, maybe they should be defined like this?
> >>
> >> + char str[INET6_ADDRSTRLEN + 2] = {0};
> >> + char buffer[INET6_ADDRSTRLEN + 2 + 2] = {0};
> >>
> >> Not sure what's the best way for this case.
> >>
> >> Looking at the example in the man page of inet_pton(3),
> INET6_ADDRSTRLEN
> >> seems enough for the str and contains a null char. The buffer can
> have
> >> a comma and a space (", ") so +2 is enough? i.e.
> >>
> >> char str[INET6_ADDRSTRLEN] = {0};
> >> char buffer[INET6_ADDRSTRLEN + 2] = {0};
> >>
> >>
> >> > + uint len = 0;
> >> > +
> >> > + buf = *bufp;
> >> > + pos = strlen(buf);
> >>
> >> Ah nicely done :)
> >>
> >> > +
> >> > + readmem(devaddr + OFFSET(net_device_ip6_ptr), KVADDR,
> >> > + &ip6_ptr, sizeof(ulong), "ip6_ptr", FAULT_ON_ERROR);
> >> > +
> >> > + if (!ip6_ptr)
> >> > + return;
> >> > +
> >> > + if (VALID_MEMBER(inet6_ifaddr_if_list)) {
> >> > + struct list_data list_data, *ld;
> >> > + ulong cnt = 0, i;
> >> > +
> >> > + ld = &list_data;
> >> > + BZERO(ld, sizeof(struct list_data));
> >> > + ld->flags |= LIST_ALLOCATE;
> >> > + ld->start = ip6_ptr + OFFSET(inet6_dev_addr_list);
> >> > + cnt = do_list(ld);
> >> > +
> >> > + for (i = 1; i < cnt; i++) {
> >> > +
> >> > + addr = ld->list_ptr[i] +
> OFFSET(inet6_ifaddr_addr);
> >>
> >> > + addr -= OFFSET(inet6_ifaddr_if_list);
> >>
> >>
> >> The above code is easy to understand, because it actually imitates the
> container_of().
> >>
> >> But if you would like to have the same style as show_net_devices_v2()
> and show_net_devices_v3(), that's also fine to me.
>
> Yes, please. I've not seen the above style (do_list and minus OFFSET)
> in crash before. Although we may go with the above, I think generally
> a new style tends to cause an unexpected result. so I'd like to use
> the common style if possible.
>
>
Thank you for the comment, Kazu.
I will post it later with the above suggestions.
Thanks.
Lianbo
> Thanks,
> Kazu
--
Crash-utility mailing list
Crash-utility@redhat.com
https://listman.redhat.com/mailman/listinfo/crash-utility
Contribution Guidelines: https://github.com/crash-utility/crash/wiki
