signed vs unsigned

Dirk Hohndel dirk at hohndel.org
Wed Mar 23 13:23:41 PDT 2016


> On Mar 23, 2016, at 1:07 PM, Robert Helling <helling at lmu.de> wrote:
> 
> Hi,
> 
>> On 23.03.2016, at 16:15, Dirk Hohndel <dirk at hohndel.org <mailto:dirk at hohndel.org>> wrote:
>> 
>> We have certain values that are unsigned because of the C standard.
>> sizeof() for example. We have other values that are semantically always
>> non-negative (temperature in K, pressure in a vessel).
>> 
>> The initial reason for doing all this "cleanup" work was to silence the
>> more than a thousand warnings one gets when building for iOS (where these
>> warnings are forced on). And in the process I tried to be smart and use
>> signed vs unsigned in ways that made sense given what the variables
>> actually were supposed to contain.
> 
> to continue in this vain, I did a git grep unsigned and found a few more cases that I would like to discuss before trying to turn this into a patch. Once more, I would propose to make everything signed unless you have a very good reason not to. I would not touch anything that does bit operations like dive computer communication, cryptographic algorithms etc for obvious reasons. But there are a few places where one could argue that „there might be a reason where one wants negative values“. These are
> 
> - duration_t contains unsigned seconds. And it is not only used for actual durations. We might run into semantic problems when we allow things to happen before the dive, but there might be reasons for that (the time a picture is taken). I see no reason to restrict these to positive and this would imply a few follow up changes.
> 
> - volumes (and similar pressures) sound like things that cannot be negative. But they can, for example in planning a dive that uses too much gas.
> 
> - depth can be negative when the planner does a step of ascent that happens to be greater than the previous depth. This is a signal of reaching the surface. 
> 
> In particular dive.c holds a number of „unsigned“’s that I would get rid of for aesthetic reasons.
> 
> What do you think?

I am torn.
The code now compiles without warnings and appears to work. So this is opening us up for more potential bugs that might be introduced. Additionally we need to check carefully if that one bit less in valid range could turn into a problem (i.e., this makes the maximum values we support in various situations half as big as they are today). I'm not sure that would be a problem anywhere, but I'm also not sure if we gain anything from making this change right now.

/D

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.subsurface-divelog.org/pipermail/subsurface/attachments/20160323/362f4638/attachment-0001.html>


More information about the subsurface mailing list