Comment 10 for bug 314048

Well Yury, I basically gave up because the code was - lets face it - a true
mess.

At least on VC++ 2005, if you use the "+=" operator it is returning an
"int", which is sometimes assigned to a short, so than you get a warning.

A reinterpret_cast only exists in C++, not in C. ccom.c is a C-file, not a
C++ and thus cannot be used.

Greetings,
Steven

2009/1/12 Yury V. Zaytsev <email address hidden>

> Hi, Steven,
>
> Please find my comments below:
>
> * Generic note: isn't it better to add a "//FIXME: Unused vars, Steven"
> when you declare unused vars to suppress the unused vars warning?
>
> Maybe some time later these issues will be dealt with in a more correct
> way, by just removing the 100% unused stuff from the function
> declarations? If you just cut out the warnings no one will notice this
> later.
>
> * Same comment here (maybe better to tag with a FIXME):
>
> === modified file 'cuneiform_src/Kern/ced/sources/main/ced_func_old.cpp'
>
> #ifdef _MSC_VER
> # pragma warning ( disable: 4505 ) // unreferenced local function has
> been removed
> #endif // _MSC_VER
>
> * You added lots of C-style casts; well, adding implicit casts is a good
> idea but please make sure it won't break on other platforms. E.g. I am
> always afraid of those (*int) casts because it might have different size
> on x64? If I am wrong, I apologize for the confusion.
>
> * for ( ;; ) is great but in essence this is the same that while(TRUE).
> Maybe the function implementation have to be rethought instead of just
> suppressing it?
>
> * I find it strange that replacing += with stuff = stuff + stuff fixes
> things. Maybe this has to be investigated further (ctb_pack.c etc.)?
>
> * ccom.c diff looks weird?! It replaces the whole file. Did you really
> change every single line? Probably this is because of the \r\n line
> endings. Let us stick to the uniform line endings.
>
> * ccom.c at the very end:
>
> #ifdef _MSC_VER
> # pragma warning (disable: 4055) // from data pointer 'void *' to
> function pointer 'my_alloc_type'
> #endif // _MSC_VER
>
> Don't you think we'd better use reinterpret_cast<> ?
>
> My argument in favor of this is that these casts can be easily grepped
> as opposed to the C-style casts which very are hard to notice. Even
> though Jussi might be right about static_cast<> vs (int) (it does not
> make sense to use them because they are harmless anyway, he says), but I
> feel that reinterpret_casts are very dangerous and well deserve it.
>
> * Same for the code below:
>
> #ifdef _MSC_VER
> # pragma warning( disable: 4054 ) // 'type cast' : from function
> pointer '...' to data pointer '...'
> # pragma warning( disable: 4055 ) // 'type cast' : from data pointer
> '...' to function pointer '...'
> #endif
>
> * crling.h, loc.c - line endings?
>
> Hope that helps and thank you for your efforts!
>
> --
> Sincerely yours,
> Yury V. Zaytsev
>
> --
> Cut down on compiler warnings
> https://bugs.launchpad.net/bugs/314048
> You received this bug notification because you are a member of Cuneiform
> Linux, which is the registrant for Cuneiform for Linux.
>
> Status in Linux port of Cuneiform: New
>
> Bug description:
> The code currently emits quite a lot of compiler warnings. Getting rid of
> these would be beneficial
>
> Attached is a patch from Steven Van Ingelgem that removes a lot of MSVC
> warnings. Unfortunately it breaks recognition completely on OS X.
>