On Thu, Nov 30, 2023 at 10:07:44AM +0100, Stefano Brivio wrote:On Thu, 30 Nov 2023 11:27:21 +1100 David Gibson <david(a)gibson.dropbear.id.au> wrote:Again, I don't think so. ptrdiff_t is important because it allows for the difference of two *unconstrained* pointers. In our case the pointer is not unconstrained, and we want to return it as whatever type we typically use for an index into the connection table, which is an unsigned int.On Wed, Nov 29, 2023 at 02:58:42PM +0100, Stefano Brivio wrote:I also think it should be unsigned (s/which is signed/which happens to be signed/ in my comment above). At the same time there's no such thing as an unsigned version of ptrdiff_t, so, given the other choices, I would still argue that we should use ptrdiff_t.On Wed, 29 Nov 2023 14:46:09 +0100 Stefano Brivio <sbrivio(a)redhat.com> wrote:And then a bunch will be obsoleted by "flow, tcp: Add logging helpers for connection related messages".On 32-bit architectures, it's a regular int. C99 introduced ptrdiff_t for this case, with a matching length modifier, 't'. Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- tcp.c | 39 +++++++++++++++++++++------------------ tcp_splice.c | 14 +++++++------- 2 files changed, 28 insertions(+), 25 deletions(-) diff --git a/tcp.c b/tcp.c index 44468ca..c32c9cb 100644 --- a/tcp.c +++ b/tcp.c @@ -727,7 +727,7 @@ static void tcp_timer_ctl(const struct ctx *c, struct tcp_tap_conn *conn) it.it_value.tv_sec = ACT_TIMEOUT; } - debug("TCP: index %li, timer expires in %lu.%03lus", CONN_IDX(conn), + debug("TCP: index %ti, timer expires in %lu.%03lus", CONN_IDX(conn), [...]Oops, I just realised this clashes with your "[PATCH v2 03/11] flow, tcp: Consolidate flow pointer<->index helpers".There, however, I guess that the new flow_idx() should return ptrdiff_t, which is signed.Actually, no, I don't think so. Yes the expression that generates it is naturally of type ptrdiff_t. But it's a bug to call flow_idx() on something not in the flow table, and places where we want to pass *in* a flow table index it makes more sense for it to be unsigned. So I think flow indices should be unsigned throughout.-- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibsonSure, dropping this.I can drop this patch if you re-spin it (assuming it makes sense to you), or I can adapt it on top of your patch -- whatever is most convenient for you.I have a couple of reasons to re-spin anyway. So how about you drop this, and I'll double check that I get the format specifiers sane after my series?