Review: Needs Fixing

In several cases where you added casts, you cast to the wrong type, e.g., the 
wrong size_type or something that should have been size_t.  The only reason it 
got rid of the warnings in those cases is because the wrong type and the 
correct type just so happened to be the same underlying type on Windows; but 
that's not necessarily the case on non-Windows platforms.

For example, in mem_streambuf.cpp, you cast size to string::size_type -- why?  
Strings have nothing to do with that code.  If you looked at the man page for 
memcpy(3), the type of the 3rd argument is clearly size_t, so that's what you 
should have cast to.

I've made corrections in my own code at least.

In my code, I prefer !!expr rather than expr != 0 for conversion to bool (saves 
typing) -- I've changed this myself.

In places where you added #ifdef __GNUC__, it would have been nice if you fixed 
the indentation and included the /* __GNUC__ */ comment on the #endif -- I've 
fixed this myself.

As for other casts, I didn't thoroughly review them (you should do that, not 
me), but they looked suspicious to me:

209: It's not clear to me that the cast should be to int. It probably ought to 
be to whatever the type of theContent's size_type is (if it has one).

227,230,239: Here you're casting to unsigned int for theContent even though you 
just cast to int on line 209.  Again, this should probably be some size_type.

252: Is unsigned long really correct for bytes_in_buffer?  Is there a 

261: You fixed this line but ignored line 259 that obviously casts to a 
different type.

274: Is int really the right type?  It probably should be theXmlTrees's 
size_type (if it has one).

283,296: Should probably be the type of the iterator's type's size_type.

322: Should probably be vector<store::Item*>::size_type not int.  (To save 
typing, just add a typedef to the class for vector<store::Item*>.)

331,357: Should probably be theItems's type's size_type not unsigned int.

Line numbers above refer to the line numbers of the diff on the launchpad merge 
Your team Zorba Coders is subscribed to branch lp:zorba.

Mailing list:
Post to     :
Unsubscribe :
More help   :

Reply via email to