Hi Marti,
Thanks for the feedback on seeing errors for interpolation and math; I'm sorry
I broke something. I was afraid that might happen, since I wasn't clear in a
couple of places on whether the math was intended to actually able to handle
negative numbers.
I already have a pretty good idea of where the problems may be. There is also
a small chance I corrected some inaccuracies, but at this point I trust the
test bed more than my own recent work.
I'll be happy to remake the changes against your latest sources from Git,
assuming I can find them, since my changes are fresh in my mind and they really
didn't take long to do.
And I do need to take the time to enable the testbed and verify my work myself.
My intent is certainly not to make additional work for you so that everyone can
gain the potential benefits of more rigorous compiler checking.
So everyone else on the mailing list:
Please at this point hold off taking the sources I posted before, at least
until I can at least pass them through the testbed tests successfully or know
exactly why they fail.
-Noel
From: Martí Maria [mailto:marti.ma...@littlecms.com]
Sent: Wed, July 26, 2017 3:50 AM
To: Noel Carboni; lcms-user@lists.sourceforge.net
Cc: 'Aaron Boxer'
Subject: Re: [Lcms-user] Build warnings on OSX / clang
Hi Noel,
Many thanks for your hard work. There are, however, two important issues:
- You modified sources other than latest ones in GIT. That means all changes
since 2.8 are lost. Changes are big, and this means merging is difficult and
error prone.
- The testbed reports multiple failures in critical parts like interpolation
and math in general. This is bad :(
I would be glad to accept some changes to silence compiler warnings, but I'm
afraid merging those all changes and make sure all is working would take me
months that currently I don't have.
Regards
Marti.
On 7/26/2017 6:26 AM, Noel Carboni wrote:
Hi guys,
I've had a pass through the code. I made several hundred changes to 20 or so
source files, and now have it building here with warnings set to max in Visual
Studio 2017. Mostly the changes were to make unsigned variables out of signed
variables, and I didn't have to add much casting.
I have tested it in my own application (which of course only exercises part of
the library), and it seems to work great. I'm not presently set up to run the
battery of tests included with the software, so I hope someone can help with
that. It would be also interesting to know if other compilers (e.g., clang)
find fault where Visual Studio does not.
Here's the source code:
http://Noel.ProDigitalSoftware.com/temp/LittleCMS_2.8NC.zip
I've made a number of changes to the release 2.8 sources with the following
philosophies in mind:
1. Don't alter the external interface (lcms2.h)
2. Minimize the changes Marti will have to review and do them as safely as
possible.
3. Make the use of signed and unsigned types consistent to reduce the hundreds
of warnings emitted when -Wall is used.
4. Use as few casts as possible.
4. Maintain efficiency.
With this set of files you can now enable -Wall in Visual Studio 2017 builds
for ongoing LittleCMS development work, to gain the benefits of tighter type
checking, but with the following caveats:
A. You'll most likely want to specifically disable several of the -Wall
warnings by putting the following additional options on the C/C++ compiler
command line:
/wd4711 /wd4820 /wd4061 /wd4774 /wd4710 /wd4668
These specifically quiet the following Visual Studio 2017 C++ warnings,
which may not be helpful to you:
warning C4711: function 'xxxxxxxxxxxxxxxxx' selected for automatic
inline expansion
warning C4820: 'xxxxxxxx': 'n' bytes padding added after data member
warning C4061: enumerator 'xxxxxxxxxxxxx' in switch of enum
'yyyyyyyyyyyyy' is not explicitly handled by a case label
warning C4774: '_snprintf' : format string expected in argument 3 is
not a string literal
warning C4710: 'int _snprintf(char *const ,const ::size_t,const char
*const ,...)': function not inlined
warning C4668: '_M_HYBRID_X86_ARM64' is not defined as a preprocessor
macro, replacing with '0' for '#if/#elif'
B. There is one warning, in cmsxform.c, that may indicate a real problem,
especially in light of compilation of this library for different systems with
different compilers. Marti, you may want to review the code and decide whether
to change it or suppress the warning:
..\..\..\src\cmsxform.c(799): warning C4191: 'type cast': unsafe
conversion from '_cmsTransform2Fn' to '_cmsTransformFn'
The above text, to help Marti see what I've done, is also in a comment block in
lcms2_internal.h.
I suggest careful review and testing of these changes should be done. I don't
think I have corrected any serious logic errors, and I hope I haven't caused
any new ones. The compiler's additional scrutiny can be brought to bear going
forward to help Marti et. al. with ongoing development.
-Noel
From: Aaron Boxer [mailto:boxe...@gmail.com]
Sent: Mon, July 24, 2017 7:03 PM
To: Marti Maria
Cc: Noel Carboni; lcms-user@lists.sourceforge.net
Subject: Re: [Lcms-user] Build warnings on OSX / clang
Thanks to Noel for offering fixes, and to Marti for graciously accepting :)
I've gone through similar issues with my JPEG 2000 codec, much of the code
inherited from developers
who didn't worry about these issues. I spent a lot of time converting signed
to unsigned for all quantities
that must always be positive : number of image components, image dimensions,
etc.
Also, removed as many casts as possible. The code is now more resilient to
overflow, and compiles with no
warnings at maximum -Wall compiler settings. I think it is worth the effort.
Regards,
Aaron
On Mon, Jul 24, 2017 at 6:31 PM, Marti Maria <marti.ma...@littlecms.com> wrote:
Sounds great. All improvements are more than welcome.
Regards
Marti
On 25 Jul 2017 00:27, Noel Carboni <ncarb...@prodigitalsoftware.com> wrote:
Hi Marti,
As C/C++ programmers we have to face the fact that the language is growing ever
more tightly typed.
It's not quite correct even to promote implicitly from signed to unsigned when
you "just know" that the values will be small, since under some conditions
negative numbers could have meaning. Or worse, be given special meaning in the
future. It's tempting to return -1 as a special error or "not found" case, but
that can ultimately cause conflict with code that expects only signed values.
It's only really properly done - and thus "safe" from future maintenance
problems - if you use consistent types across the board, or add casts
occasionally when you really do want to express that you're fitting a small
positive unsigned value into a signed field.
Looking your released code over just now I think that it can be substantially
cleaned up without a lot of casts added. There are a lot of cases where you
have a habit of using "int" when what you really want is "unsigned int" (e.g.,
for number of channels, where negative values don't have meaning). :)
Tell you what; I'll go through all the code and see what I can come up with to
get it down to zero warnings at max warning level. I don't think it'll take me
more than a few man-hours. I'll send the result to you and you can compare the
files and see whether you feel the changes are safe enough. I'll make it my
mission not to change the logic or your intent with the style.
-Noel
From: Marti Maria [mailto:marti.ma...@littlecms.com]
Sent: Mon, July 24, 2017 6:09 PM
To: Noel Carboni
Cc: lcms-user@lists.sourceforge.net
Subject: Re: [Lcms-user] Build warnings on OSX / clang
Hi,
Sometimes I use signed to unsigned promotion without cast. This is safe if
values are small and positive. Mostly to avoid casts in memset, memmove,
calloc, etc. which looks really ugly.
If you can found other cases, like that one in cmsgamma.c, please let me know.
But this latter was fixed after the original report.
Regards
Marti
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Lcms-user mailing list
Lcms-user@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/lcms-user
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Lcms-user mailing list
Lcms-user@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/lcms-user
