On Thu, Jul 14, 2011 at 1:37 PM, Gregory Farnum
<gregory.farnum@xxxxxxxxxxxxx> wrote:
> I've been going over the Google Coding Style document and noticed a
> few other things that probably merit discussion/a diff:
> 1) It says not to use streams, except for logging. We don't do a lot
> of text output other than logging, but in the few places we do it
> (program help text, for instance) I believe we use stream.s
I don't think it's really worth changing all the program help text
output to use fprintf. cout and cerr are fine enough for that.
On a semi-related note, I never liked std::ifstream and std::ofstream
because I feel like proper error handling was more awkward with these
than with fprintf. The streams don't even return an error code, they
just set an error bit which you are supposed to periodically check. So
it might be worth using fprintf in these cases.
> 2) Under the "Integer Types" section, they prefer not to use unsigned
> types except for bit patterns. They explicitly state that values which
> are not allowed to be <0 should be checked with asserts, rather than
> guaranteed by the type. This isn't the case for any of our current
> code and I'm not sure it's worth changing.
The rationale is that signed/unsigned bugs can occur pretty frequently
because of the way C/C++ handles type promotion.
For example, what does this print?
> unsigned int foo = 2;
> int bar = -1;
> if (foo < bar)
> printf("foo < bar\n");
Well, bar gets promoted to unsigned int, making it the largest
possible unsigned int.
That's why GCC now has the "comparison between signed/unsigned"
warning. But even with this active, mixing signed and unsigned forces
you to litter your code with typecasts to avoid the warning, which is
not ideal.
So I sort of see where they're coming from on that. I don't think it's
worth changing our current code, and there are some places where I
really think we should use unsigned in the future, but in general, if
you don't need it, don't use it.
cheers,
Colin
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
