Re: 100+ warnings and some cosmetics along the way (issue1724041)

[Han-Wen Nienhuys]

On Thu, Jun 17, 2010 at 9:55 AM, Pap Lôrinc <address@hidden> wrote:
>  - it *is* cosmetics, it's just that the unmarked casting to int made it
> seemingly possible to be < 0, which it cannot be, since fm->name_to_index
> (key) returns an unsigned positive value anyway


#include <stdio.h>

main() {
  unsigned x = -1;
  int i = (int) x;
  printf("%d\n", i);
}

prints

-1

I gladly concede that this code is ugly and unintuitive (I suspect an
example of blindly making array indices unsigned), but just removing
the check is not the right solution, and would probably make the
program crash when using note-head styles that do not have a glyphs
for every duration.  (Did you run the regression test?)

> size_t
> Font_metric::name_to_index (string) const
> {
>   return (size_t)-1;
> }
>
> (and it doesn't even depend
> on the key, which is quite strange to me).

It is a virtual method, and this is the default implementation.

I dont want to knock down your hard and meticulous work; we are in
need of code janitoring, but I ask that you submit the changes in a
separated way: it's easier to review, and if it does break something,
it's easier to identify and revert the incriminating commit.

I recommend the following:  apply your patch on a clean tree, then use

 git add --interactive --patch

to select all patch hunks that have 60 -> 80 column changes, then
commit, then select all hunks that have comment typo changes, etc.

In the end, you should have multiple commits each addressing a
separate topic, which we can discuss separately.

-- 
Han-Wen Nienhuys - address@hidden - http://www.xs4all.nl/~hanwen