Re: [PATCH v2 03/10] test-pmd: fix printf format string mismatch

[Andre Muezerie <andremue@linux.microsoft.com>]

On Tue, Feb 18, 2025 at 04:46:56PM +0000, Bruce Richardson wrote:
> On Tue, Feb 18, 2025 at 08:32:02AM -0800, Andre Muezerie wrote:
> > Compiling with MSVC results in warnings like the one below:
> > 
> > app/test-pmd/csumonly.c(1085): warning C4477: 'printf' : format string
> >     '%d' requires an argument of type 'int',
> >     but variadic argument 1 has type 'uint64_t'
> > 
> > Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
> > Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> > ---
> >  app/test-pmd/csumonly.c | 23 ++++++++++++-----------
> >  1 file changed, 12 insertions(+), 11 deletions(-)
> > 
> > diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
> > index d77a140641..8de5ad6ad9 100644
> > --- a/app/test-pmd/csumonly.c
> > +++ b/app/test-pmd/csumonly.c
> > @@ -1070,7 +1070,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
> >  				info.l2_len, rte_be_to_cpu_16(info.ethertype),
> >  				info.l3_len, info.l4_proto, info.l4_len, buf);
> >  			if (rx_ol_flags & RTE_MBUF_F_RX_LRO)
> > -				printf("rx: m->lro_segsz=%u\n", m->tso_segsz);
> > +				printf("rx: m->lro_segsz=%" PRIu64 "\n", (uint64_t)m->tso_segsz);
> 
> tso_segsz is already uint64_t, so no need for the cast.

The compilers differ in behavior here. tso_segsz only uses 16 bits of the uint64_t,
and gcc tries to be smart about it and implicitly converts tso_segsz into an int
(since it fits into an int). Msvc does not do that, and keeps the type for tso_segsz
as uint64_t. To support both compilers it seems there's no way to avoid the cast.

> 
> >  			if (info.is_tunnel == 1)
> >  				printf("rx: outer_l2_len=%d outer_ethertype=%x "
> >  					"outer_l3_len=%d\n", info.outer_l2_len,
> > @@ -1082,28 +1082,29 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
> >  					    RTE_ETH_TX_OFFLOAD_TCP_CKSUM |
> >  					    RTE_ETH_TX_OFFLOAD_SCTP_CKSUM)) ||
> >  				info.tso_segsz != 0)
> > -				printf("tx: m->l2_len=%d m->l3_len=%d "
> > -					"m->l4_len=%d\n",
> > -					m->l2_len, m->l3_len, m->l4_len);
> > +				printf("tx: m->l2_len=%" PRIu64 " m->l3_len=%" PRIu64
> > +					" m->l4_len=%" PRIu64 "\n",
> > +					(uint64_t)m->l2_len, (uint64_t)m->l3_len,
> > +					(uint64_t)m->l4_len);
> 
> Same here, using casts and a changed print format seems wrong in the patch.
> Either we change the format string to match the variable type, or we cast
> the variables to match the format string. We should not do both, IMHO.

I also normally prefer to not see unnecessary casts in the code, but it looks like
it is needed in this case, as explained above.

> 
> >  			if (info.is_tunnel == 1) {
> >  				if ((tx_offloads &
> >  				    RTE_ETH_TX_OFFLOAD_OUTER_IPV4_CKSUM) ||
> >  				    (tx_offloads &
> >  				    RTE_ETH_TX_OFFLOAD_OUTER_UDP_CKSUM) ||
> >  				    (tx_ol_flags & RTE_MBUF_F_TX_OUTER_IPV6))
> > -					printf("tx: m->outer_l2_len=%d "
> > -						"m->outer_l3_len=%d\n",
> > -						m->outer_l2_len,
> > -						m->outer_l3_len);
> > +					printf("tx: m->outer_l2_len=%" PRIu64
> > +						" m->outer_l3_len=%" PRIu64 "\n",
> > +						(uint64_t)m->outer_l2_len,
> > +						(uint64_t)m->outer_l3_len);
> >  				if (info.tunnel_tso_segsz != 0 &&
> >  						(m->ol_flags & (RTE_MBUF_F_TX_TCP_SEG |
> >  							RTE_MBUF_F_TX_UDP_SEG)))
> > -					printf("tx: m->tso_segsz=%d\n",
> > -						m->tso_segsz);
> > +					printf("tx: m->tso_segsz=%" PRIu64 "\n",
> > +						(uint64_t)m->tso_segsz);
> >  			} else if (info.tso_segsz != 0 &&
> >  					(m->ol_flags & (RTE_MBUF_F_TX_TCP_SEG |
> >  						RTE_MBUF_F_TX_UDP_SEG)))
> > -				printf("tx: m->tso_segsz=%d\n", m->tso_segsz);
> > +				printf("tx: m->tso_segsz=%" PRIu64 "\n", (uint64_t)m->tso_segsz);
> >  			rte_get_tx_ol_flag_list(m->ol_flags, buf, sizeof(buf));
> >  			printf("tx: flags=%s", buf);
> >  			printf("\n");
> > -- 
> > 2.48.1.vfs.0.0
> >