> Subject: Re: [PATCH v7 02/25] ethdev: add a link status text representation
>
> 10.07.2020 16:06, Yigit, Ferruh пишет:
> > On 7/10/2020 8:02 AM, Ivan Dyukov wrote:
> >> This commit add function which treat link status structure and format
> >> it to text representation.
> >>
> >> Signed-off-by: Ivan Dyukov <i.dyu...@samsung.com>
> > <...>
> >
> >> +static int
> >> +rte_eth_link_strf_parser(char *str, size_t len, const char *const fmt,
> >> + const struct rte_eth_link *link) {
> >> + size_t offset = 0;
> >> + const char *fmt_cur = fmt;
> >> + char *str_cur = str;
> >> + double gbits = (double)link->link_speed / 1000.;
> >> + static const char autoneg_str[] = "Autoneg";
> >> + static const char fixed_str[] = "Fixed";
> >> + static const char fdx_str[] = "FDX";
> >> + static const char hdx_str[] = "HDX";
> >> + static const char unknown_str[] = "Unknown";
> >> + static const char up_str[] = "Up";
> >> + static const char down_str[] = "Down";
> >> + char gbits_str[20];
> >> + char mbits_str[20];
> >> +
> >> + /* preformat complex formatting to easily concatinate it further */
> >> + snprintf(mbits_str, sizeof(mbits_str), "%u", link->link_speed);
> >> + snprintf(gbits_str, sizeof(gbits_str), "%.1f", gbits);
> >> + /* init str before formatting */
> >> + str[0] = 0;
> >> + while (*fmt_cur) {
> >> + /* check str bounds */
> >> + if (offset > (len - 1)) {
> >> + str[len - 1] = '\0';
> >> + return -1;
> >> + }
> >> + if (*fmt_cur == '%') {
> >> + /* set null terminator to current position,
> >> + * it's required for strlcat
> >> + */
> >> + *str_cur = '\0';
> >> + switch (*++fmt_cur) {
> >> + /* Speed in Mbits/s */
> >> + case 'M':
> >> + if (link->link_speed ==
> >> + ETH_SPEED_NUM_UNKNOWN)
> >> + offset = strlcat(str, unknown_str,
> >> + len);
> >> + else
> >> + offset = strlcat(str, mbits_str, len);
> >> + break;
> >> + /* Speed in Gbits/s */
> >> + case 'G':
> >> + if (link->link_speed ==
> >> + ETH_SPEED_NUM_UNKNOWN)
> >> + offset = strlcat(str, unknown_str,
> >> + len);
> >> + else
> >> + offset = strlcat(str, gbits_str, len);
> >> + break;
> >> + /* Link status */
> >> + case 'S':
> >> + offset = strlcat(str, link->link_status ?
> >> + up_str : down_str, len);
> >> + break;
> >> + /* Link autoneg */
> >> + case 'A':
> >> + offset = strlcat(str, link->link_autoneg ?
> >> + autoneg_str : fixed_str, len);
> >> + break;
> >> + /* Link duplex */
> >> + case 'D':
> >> + offset = strlcat(str, link->link_duplex ?
> >> + fdx_str : hdx_str, len);
> >> + break;
> >> + /* ignore unknown specifier */
> >> + default:
> >> + *str_cur = '%';
> >> + offset++;
> >> + fmt_cur--;
> >> + break;
> > What do you think ignoring the unknown specifiers and keep continue
> > processing the string, instead of break? Just keep unknown specifier
> > as it is in the output string.
> yep. it's exactly what code do. break exit from the switch but not from
> string
> processing. I have unit tests for this case. They work fine.
> Please review unit tests and send me more cases if they need to be tested.
yes it does as you explained, I miss read it, so it is good, please ignore this
comment.
