On 10 February 2012 04:13,  <[email protected]> wrote:
> No functional changes.  Mainly cosmetic changes to TrackButtons.  A couple
> of int's changed to the more sensible unsigned.

I feel bad about popping up only to quibble about something, but
changing int to unsigned on the whole is not a very good idea --
rather the opposite.

The main problem is overflow when computed values used for e.g. loop
indices unexpectedly become negative (that is, large and positive in
the unsigned world). For example,

  for (n = 0; n < m - 1; ++n) { ... }
  while (i < k - j) { ... }

will loop rather more than expected if m == 0 or j > k.

This is surprisingly common: it's one of the biggest sources of bugs I
see in code I get to review at work. It's more common in code that
does mathematical or signal-processing work than in most of
Rosegarden, but I'm sure it isn't the case that this never happens in
Rosegarden code.

I do understand that there are sound reasons one might want to use
unsigneds, and they're not only related to getting an extra bit for
the maximum positive value. For instance, unsigned overflow is defined
while signed overflow is not: UINT_MAX + 1 == 0, while INT_MAX + 1 is
undefined. But in practice, in most places where ints get used, signed
overflow is far less likely to be an issue than any problems with
unsigned arithmetic. (As an aside, this detail of the behaviour of
signed arithmetic allows the compiler to do a number of useful
optimisations if you happen to use signeds for loop counters, because
it is allowed to assume they never overflow.)

Also, of course, the standard library uses unsigneds for things like
vector sizes (although you'll notice that the Qt API always uses plain
int) so it's tempting to make everything else unsigned to avoid
compiler warnings and the like. Quite honestly though, it's far safer
to do the opposite and convert the unsigned to a signed int before you
use it.

Using signeds for values that are "philosophically" unsigned can also
have useful side-effects, such as in this case being able to use -1
for "unset" or "no value", more meaningful to the reader than
UINT_MAX.

Unless you're explicitly representing a bit-addressable memory region
or something like that, I'd seriously suggest avoiding unsigned
altogether. I used to use unsigned and size_t quite a bit, and since I
realised how error-prone it was (because like most programmers, I'm
just not really good enough) I've pretty much eliminated them from my
code with only good consequences.

(This has come up on this list before, I think. Last time I had this
discussion with someone, I was amused to see this commit in one of his
projects a few weeks later:
http://code.soundsoftware.ac.uk/projects/audiodb/repository/revisions/6a5117d68ac4)


Chris

------------------------------------------------------------------------------
Virtualization & Cloud Management Using Capacity Planning
Cloud computing makes use of virtualization - but cloud computing 
also focuses on allowing computing to be delivered as a service.
http://www.accelacomm.com/jaw/sfnl/114/51521223/
_______________________________________________
Rosegarden-devel mailing list
[email protected] - use the link below to unsubscribe
https://lists.sourceforge.net/lists/listinfo/rosegarden-devel

Reply via email to