Opened 13 days ago

Closed 11 days ago

#24861 closed defect (fixed)

Using %zu seems to break mingw :/

Reported by: nickm Owned by: ffmancera
Priority: High Milestone: Tor: 0.3.3.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: mingw, compatibility, regression, review-group-29
Cc: ffmancera Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

It looks like we found out why we weren't using %zu before:

17:55:39 In file included from src/or/or.h:72:0,
17:55:39                  from src/or/circuitlist.c:54:
17:55:39 src/or/circuitlist.c: In function 'circuits_handle_oom':
17:55:39 src/or/circuitlist.c:2407:26: error: unknown conversion type character 'z' in format [-Werror=format=]
17:55:39    log_notice(LD_GENERAL, "We're low on memory (cell queues total alloc: %zu,"
17:55:39                           ^
17:55:39 ./src/common/torlog.h:232:45: note: in definition of macro 'log_notice'
17:55:39    log_fn_(LOG_NOTICE, domain, __FUNCTION__, args, ##__VA_ARGS__)
17:55:39                                              ^~~~
17:55:39 src/or/circuitlist.c:2407:26: error: unknown conversion type character 'z' in format [-Werror=format=]
17:55:39    log_notice(LD_GENERAL, "We're low on memory (cell queues total alloc: %zu,"
17:55:39                           ^

(from https://jenkins.torproject.org/job/tor-ci-mingwcross-master/1577/ARCHITECTURE=amd64,SUITE=stretch/consoleFull)

In theory we can select a more c99 one with __USE_MINGW_ANSI_STDIO, but that would change our stdio everywhere. (Which is a little scary.)

We could also define a PRIsz macro that has the correct format for whatever compiler we are using. That might be a better choice.

Child Tickets

Change History (8)

comment:1 Changed 13 days ago by ffmancera

Owner: set to ffmancera
Status: newassigned

comment:2 Changed 13 days ago by catalyst

We might want to avoid using PRIuSZ or anything too similar because macros of that shape are reserved in C99's "Future library directions" for inttypes.h. Maybe TOR_PRIuSZ? Or is that too long?

comment:3 Changed 13 days ago by nickm

That's probably fine.

comment:4 Changed 12 days ago by ffmancera

Status: assignedneeds_review

Patch done but I didn't test it on minGW, so It would be cool if someone could test it properly.

Please check my github branch bug24861. I hope everything is fine! :)

comment:5 Changed 12 days ago by nickm

Keywords: review-group-29 added

comment:6 Changed 11 days ago by nickm

Status: needs_reviewneeds_revision

quick review: This looks good, but other files will want to use this too. Let's move the TOR_PRIuSZ definition to torint.h

comment:7 Changed 11 days ago by ffmancera

Status: needs_revisionneeds_review

Changes done! Let's see if everything is fine now!

comment:8 Changed 11 days ago by nickm

Resolution: fixed
Status: needs_reviewclosed

merged! Let's see if the builders like it.

Note: See TracTickets for help on using tickets.