On Mon, 6 Jul 2020 23:37:16 +0300
Ivan Dyukov <i.dyu...@samsung.com> wrote:
> static int
> +rte_eth_link_strf_parser(char *str, size_t len, const char *const fmt,
> + struct rte_eth_link *link)
The link arg should be const.
> +{
> + 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";
Why are these UPPER_CASE_CONSTANTS? That does not match DPDK style.
> +
> + char gbits_str[20];
> + char mbits_str[20];
> + /* preformat complex formatting to easily concatinate it further */
Blank line between declaration and code.
> + snprintf(mbits_str, 20, "%u", link->link_speed);
> + snprintf(gbits_str, 20, "%.1f", gbits);
Use sizeof(mbits_str) which is safer and matches what youare doing.
> + /* 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;
> + /* Error cases */
> + default:
> + return -1;
> +
> + }
> + if (offset > (len - 1))
> + return -1;
> +
> + str_cur = str + offset;
> + } else {
> + *str_cur++ = *fmt_cur;
> + offset++;
> + }
> + fmt_cur++;
> + }
> + *str_cur = '\0';
> + return offset;
> +}
> +
> +int
> +rte_eth_link_printf(const char *const fmt,
> + struct rte_eth_link *link)
> +{
> + char text[200];
> + int ret;
> + ret = rte_eth_link_strf(text, 200, fmt, link);
Blank line tween declaration and code.
> + if (ret > 0)
> + printf("%s", text);
> + return ret;
> +}
