> On Mar 23, 2016, at 1:07 PM, Robert Helling <[email protected]> wrote: > > Hi, > >> On 23.03.2016, at 16:15, Dirk Hohndel <[email protected] >> <mailto:[email protected]>> 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
_______________________________________________ subsurface mailing list [email protected] http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
