Signed/unsigned mismatch

rendservice.[ch] are mixing signed and unsigned types for struct rend_intro_cell_s member ciphertext_len.

added component::core tor/tor priority::medium resolution::fixed severity::normal status::closed type::defect labels

Using clang 3.4.1:

  CC       src/or/rendservice.o
/usr/local/src/tor/src/or/rendservice.c:2027:29: error: comparison of integers of different signs: 'ssize_t'
      (aka 'long') and 'size_t' (aka 'unsigned long') [-Werror,-Wsign-compare]
    parsed_req->ciphertext, MIN(parsed_req->ciphertext_len, keylen),
                            ^   ~~~~~~~~~~~~~~~~~~~~~~~~~~  ~~~~~~
/usr/include/sys/param.h:298:23: note: expanded from macro 'MIN'
#define MIN(a,b) (((a)<(b))?(a):(b))

See branch bug24480 in my public repo for a fix.

Trac:
Status: new to needs_review

Bug confirmed.

For the fix, how about we just change keylen to ssize_t instead? It is a smaller and much less risky change, since it's only a local variable, and only used in one place.

Example patch:

diff --git a/src/or/rendservice.c b/src/or/rendservice.c
index ba8891eade39ea..80e1e10a054b4a 100644
--- a/src/or/rendservice.c
+++ b/src/or/rendservice.c
@@ -1162,7 +1162,7 @@ rend_service_introduce(origin_circuit_t *circuit, const uint8_t *request,
   time_t now = time(NULL);
   time_t elapsed;
   int replay;
-  size_t keylen;
+  ssize_t keylen;
 
   /* Do some initial validation and logging before we parse the cell */
   if (circuit->base_.purpose != CIRCUIT_PURPOSE_S_INTRO) {

What do you think?

Changing size_t keylen to ssize_t keylen looks good to me.

Trac:
Status: needs_review to merge_ready

Added a changes file and pushed as 461e34bb3dec0f4f7234584806a224040f44c7ed. Thanks!

Trac:
Status: merge_ready to closed
Resolution: N/A to fixed

Replying to nickm:

What do you think?

I agree that's much better. Thanks.

closed

mentioned in issue #24568 (moved)

moved to tpo/core/tor#24480 (closed)