Bug 1205533 - Fix and disallow warnings in gfx/qcms/. r=jrmuizel.
authorNicholas Nethercote <nnethercote@mozilla.com>
Thu, 17 Sep 2015 17:11:27 -0700
changeset 295775 4f8368eec32d8cbbef12297441da0e00e041cc50
parent 295774 d63692ee53300a39d85b75378fcc24d115777639
child 295776 c7cc8e32b62f7f9f15c6c8972e705e947b361ca9
push id5245
push userraliiev@mozilla.com
push dateThu, 29 Oct 2015 11:30:51 +0000
treeherdermozilla-beta@dac831dc1bd0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjrmuizel
bugs1205533
milestone43.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1205533 - Fix and disallow warnings in gfx/qcms/. r=jrmuizel. This patch fixes various warnings from MSVC. - Several "truncation from 'double' to 'float'" warnings, easily fixed by appending 'f' to literals. - Some "signed/unsigned mismatch" warnings. In read_tag_lutType(), MSVC is apparently promoting the multiplication of a uint8_t and a uint16_t to an int32_t, oddly enough. A uint32_t cast fixes the warning. - |offset| was unused in qcms_data_create_rbg_with_gamma(). - A couple of "overflow in floating-point constant arithmetic" warnings involving INFINITY in transform_util.c. There is some type confusion here -- in C99 HUGE_VAL is a double and INFINITY is a float. So the HUGE_VAL here should actualy be HUGE_VALF. But, strangely enough, that isn't enough to avoid the warning, I don't know why. However, it turns out that any non-positive value for |interval| will have the same effect, so I just removed all the INFINITY/HUGE_VAL stuff and used -1 instead. It also fixes an ARM-only GCC warning. - "'__force_align_arg_pointer__' attribute directive ignored". This is an x86-only attribute. Instead of disabling it on x86-64, instead enable it on i386 (which avoids enabling it uselessly on ARM).
gfx/qcms/iccread.c
gfx/qcms/moz.build
gfx/qcms/transform.c
gfx/qcms/transform_util.c
--- a/gfx/qcms/iccread.c
+++ b/gfx/qcms/iccread.c
@@ -307,27 +307,27 @@ qcms_bool qcms_profile_is_bogus(qcms_pro
 
 
        // Sum the values; they should add up to something close to white
        sum[0] = rX + gX + bX;
        sum[1] = rY + gY + bY;
        sum[2] = rZ + gZ + bZ;
 
        // Build our target vector (see mozilla bug 460629)
-       target[0] = 0.96420;
-       target[1] = 1.00000;
-       target[2] = 0.82491;
+       target[0] = 0.96420f;
+       target[1] = 1.00000f;
+       target[2] = 0.82491f;
 
        // Our tolerance vector - Recommended by Chris Murphy based on
        // conversion from the LAB space criterion of no more than 3 in any one
        // channel. This is similar to, but slightly more tolerant than Adobe's
        // criterion.
-       tolerance[0] = 0.02;
-       tolerance[1] = 0.02;
-       tolerance[2] = 0.04;
+       tolerance[0] = 0.02f;
+       tolerance[1] = 0.02f;
+       tolerance[2] = 0.04f;
 
        // Compare with our tolerance
        for (i = 0; i < 3; ++i) {
            if (!(((sum[i] - tolerance[i]) <= target[i]) &&
                  ((sum[i] + tolerance[i]) >= target[i])))
                return true;
        }
 
@@ -746,17 +746,17 @@ static struct lutType *read_tag_lutType(
 	lut->e02 = read_s15Fixed16Number(src, offset+20);
 	lut->e10 = read_s15Fixed16Number(src, offset+24);
 	lut->e11 = read_s15Fixed16Number(src, offset+28);
 	lut->e12 = read_s15Fixed16Number(src, offset+32);
 	lut->e20 = read_s15Fixed16Number(src, offset+36);
 	lut->e21 = read_s15Fixed16Number(src, offset+40);
 	lut->e22 = read_s15Fixed16Number(src, offset+44);
 
-	for (i = 0; i < lut->num_input_table_entries * in_chan; i++) {
+	for (i = 0; i < (uint32_t)(lut->num_input_table_entries * in_chan); i++) {
 		if (type == LUT8_TYPE) {
 			lut->input_table[i] = uInt8Number_to_float(read_uInt8Number(src, offset + 52 + i * entry_size));
 		} else {
 			lut->input_table[i] = uInt16Number_to_float(read_uInt16Number(src, offset + 52 + i * entry_size));
 		}
 	}
 
 	clut_offset = offset + 52 + lut->num_input_table_entries * in_chan * entry_size;
@@ -768,17 +768,17 @@ static struct lutType *read_tag_lutType(
 		} else {
 			lut->clut_table[i+0] = uInt16Number_to_float(read_uInt16Number(src, clut_offset + i*entry_size + 0));
 			lut->clut_table[i+1] = uInt16Number_to_float(read_uInt16Number(src, clut_offset + i*entry_size + 2));
 			lut->clut_table[i+2] = uInt16Number_to_float(read_uInt16Number(src, clut_offset + i*entry_size + 4));
 		}
 	}
 
 	output_offset = clut_offset + clut_size * out_chan * entry_size;
-	for (i = 0; i < lut->num_output_table_entries * out_chan; i++) {
+	for (i = 0; i < (uint32_t)(lut->num_output_table_entries * out_chan); i++) {
 		if (type == LUT8_TYPE) {
 			lut->output_table[i] = uInt8Number_to_float(read_uInt8Number(src, output_offset + i*entry_size));
 		} else {
 			lut->output_table[i] = uInt16Number_to_float(read_uInt16Number(src, output_offset + i*entry_size));
 		}
 	}
 
 	return lut;
@@ -1305,17 +1305,17 @@ void qcms_data_from_unicode_path(const w
 * this is a hardcode step just for "create_rgb_with_gamma", it is the only
 * requirement till now, maybe we can make this method more general in future,
 *
 * NOTE : some of the parameters below are hardcode, please refer to the ICC documentation.
 */
 #define ICC_PROFILE_HEADER_LENGTH 128
 void qcms_data_create_rgb_with_gamma(qcms_CIE_xyY white_point, qcms_CIE_xyYTRIPLE primaries, float gamma, void **mem, size_t *size)
 {
-	uint32_t length, offset, index, xyz_count, trc_count;
+	uint32_t length, index, xyz_count, trc_count;
 	size_t tag_table_offset, tag_data_offset;
 	void *data;
 	struct matrix colorants;
 
 	uint32_t TAG_XYZ[3] = {TAG_rXYZ, TAG_gXYZ, TAG_bXYZ};
 	uint32_t TAG_TRC[3] = {TAG_rTRC, TAG_gTRC, TAG_bTRC};
 
 	if ((mem == NULL) || (size == NULL))
--- a/gfx/qcms/moz.build
+++ b/gfx/qcms/moz.build
@@ -12,19 +12,16 @@ EXPORTS += [
 SOURCES += [
     'chain.c',
     'iccread.c',
     'matrix.c',
     'transform.c',
     'transform_util.c',
 ]
 
-# XXX: We should fix these warnings.
-ALLOW_COMPILER_WARNINGS = True
-
 FINAL_LIBRARY = 'xul'
 
 if CONFIG['GNU_CC']:
     CFLAGS += ['-Wno-missing-field-initializers']
 
 use_sse1 = False
 use_sse2 = False
 use_altivec = False
--- a/gfx/qcms/transform.c
+++ b/gfx/qcms/transform.c
@@ -254,19 +254,19 @@ compute_chromatic_adaption(struct CIE_XY
 
 /* from lcms: cmsAdaptionMatrix */
 // Returns the final chrmatic adaptation from illuminant FromIll to Illuminant ToIll
 // Bradford is assumed
 static struct matrix
 adaption_matrix(struct CIE_XYZ source_illumination, struct CIE_XYZ target_illumination)
 {
 	struct matrix lam_rigg = {{ // Bradford matrix
-	                         {  0.8951,  0.2664, -0.1614 },
-	                         { -0.7502,  1.7135,  0.0367 },
-	                         {  0.0389, -0.0685,  1.0296 }
+	                         {  0.8951f,  0.2664f, -0.1614f },
+	                         { -0.7502f,  1.7135f,  0.0367f },
+	                         {  0.0389f, -0.0685f,  1.0296f }
 	                         }};
 	return compute_chromatic_adaption(source_illumination, target_illumination, lam_rigg);
 }
 
 /* from lcms: cmsAdaptMatrixToD50 */
 static struct matrix adapt_matrix_to_D50(struct matrix r, qcms_CIE_xyY source_white_pt)
 {
 	struct CIE_XYZ Dn;
@@ -1389,17 +1389,17 @@ qcms_transform* qcms_transform_create(
 	} else {
 		assert(0 && "unexpected colorspace");
 		qcms_transform_release(transform);
 		return NULL;
 	}
 	return transform;
 }
 
-#if defined(__GNUC__) && !defined(__x86_64__) && !defined(__amd64__)
+#if defined(__GNUC__) && defined(__i386__)
 /* we need this to avoid crashes when gcc assumes the stack is 128bit aligned */
 __attribute__((__force_align_arg_pointer__))
 #endif
 void qcms_transform_data(qcms_transform *transform, void *src, void *dest, size_t length)
 {
 	transform->transform_fn(transform, src, dest, length);
 }
 
--- a/gfx/qcms/transform_util.c
+++ b/gfx/qcms/transform_util.c
@@ -1,21 +1,15 @@
-#define _ISOC99_SOURCE  /* for INFINITY */
-
 #include <math.h>
 #include <assert.h>
 #include <string.h> //memcpy
 #include "qcmsint.h"
 #include "transform_util.h"
 #include "matrix.h"
 
-#if !defined(INFINITY)
-#define INFINITY HUGE_VAL
-#endif
-
 #define PARAMETRIC_CURVE_TYPE 0x70617261 //'para'
 
 /* value must be a value between 0 and 1 */
 //XXX: is the above a good restriction to have?
 // the output range of this functions is 0..1
 float lut_interp_linear(double input_value, uint16_t *table, int length)
 {
 	int upper, lower;
@@ -126,17 +120,17 @@ void compute_curve_gamma_table_type_para
         float a, b, c, e, f;
         float y = parameter[0];
         if (count == 0) {
                 a = 1;
                 b = 0;
                 c = 0;
                 e = 0;
                 f = 0;
-                interval = -INFINITY;
+                interval = -1;
         } else if(count == 1) {
                 a = parameter[1];
                 b = parameter[2];
                 c = 0;
                 e = 0;
                 f = 0;
                 interval = -1 * parameter[2] / parameter[1];
         } else if(count == 2) {
@@ -162,22 +156,22 @@ void compute_curve_gamma_table_type_para
                 interval = parameter[4];
         } else {
                 assert(0 && "invalid parametric function type.");
                 a = 1;
                 b = 0;
                 c = 0;
                 e = 0;
                 f = 0;
-                interval = -INFINITY;
-        }       
+                interval = -1;
+        }
         for (X = 0; X < 256; X++) {
                 if (X >= interval) {
-                        // XXX The equations are not exactly as definied in the spec but are
-                        //     algebraic equivilent.
+                        // XXX The equations are not exactly as defined in the spec but are
+                        //     algebraically equivalent.
                         // TODO Should division by 255 be for the whole expression.
                         gamma_table[X] = clamp_float(pow(a * X / 255. + b, y) + c + e);
                 } else {
                         gamma_table[X] = clamp_float(c * X / 255. + f);
                 }
         }
 }