Opened 8 years ago
Last modified 3 years ago
#4229 new defect
smartlist_len returns a (signed) int
Reported by: | rransom | Owned by: | |
---|---|---|---|
Priority: | High | Milestone: | Tor: unspecified |
Component: | Core Tor/Tor | Version: | |
Severity: | Normal | Keywords: | tor-client refactoring c correctness |
Cc: | mansourmoufid@… | Actual Points: | |
Parent ID: | Points: | 10 | |
Reviewer: | Sponsor: |
Description
smartlist_len(sl)
has type int
. It shouldn't be signed.
Child Tickets
Attachments (1)
Change History (10)
comment:1 Changed 8 years ago by
Milestone: | Tor: 0.2.2.x-final → Tor: 0.2.3.x-final |
---|
comment:2 follow-up: 3 Changed 8 years ago by
It's also not quite clear what we want to do for lengths in general -- do we sometimes use negative return values to indicate failure of a function that would normally return a length? At the very least, we need safe (overflow-checking) unsigned-to-signed and signed-to-unsigned cast macros.
Changed 8 years ago by
comment:3 Changed 8 years ago by
Replying to rransom:
It's also not quite clear what we want to do for lengths in general -- do we sometimes use negative return values to indicate failure of a function that would normally return a length? At the very least, we need safe (overflow-checking) unsigned-to-signed and signed-to-unsigned cast macros.
One approach is to pass a pointer to where "returned" lengths will be stored as an argument, and use the return values to indicate errors only.
See attached for an example.
comment:4 Changed 8 years ago by
Cc: | mansourmoufid@… added |
---|
comment:5 Changed 8 years ago by
If we're going to change the return type of smartlist_len() {or even if we're not}, we need to be prepared to audit all 448 places that uses it, to make sure it behaves right given an unsigned or a size_t or whatever.
As a stopgap, perhaps we should just ensure that a smartlist's size can't grow larger than INT_MAX ?
comment:6 Changed 8 years ago by
Milestone: | Tor: 0.2.3.x-final → Tor: unspecified |
---|
At the moment, actually, thanks to the smartlist_ensure_capacity fixes, we have the property that the capacity of a smartlist can never be more than INT_MAX . So it's okay to return an int for smartlist_len. I'm throwing this out of 0.2.3.x; it's a big refactoring, and I'm pretty sure that we don't actually *want* to allow a smartlist to have anything close to INT_MAX entries.
comment:7 Changed 7 years ago by
Keywords: | tor-client added |
---|
comment:8 Changed 7 years ago by
Component: | Tor Client → Tor |
---|
comment:9 Changed 3 years ago by
Keywords: | refactoring c correctness added |
---|---|
Points: | → 10 |
Severity: | → Normal |
I don't think this can be fixed on 0.2.2.x. (We would have too many merge conflicts.)