Closed Bug 523848 Opened 11 years ago Closed 11 years ago

qcms typedef clashes on AIX with <sys/types.h>

Categories

(Core :: GFX: Color Management, defect)

PowerPC
AIX
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9.3a1

People

(Reporter: ul-mcamafia, Assigned: shailen.n.jain)

Details

Attachments

(1 file, 5 obsolete files)

User-Agent:       Mozilla/5.0 (X11; U; AIX 5.1; en-US; rv:1.8.1.23) Gecko/20091019 SeaMonkey/1.1.18
Build Identifier: MOZILLA 1.9.1.4 / Seamonkey 2.0 rc2

Several compilation errors in the qcms module for AIX platform.
1.) give compilation flag -qenum=small in Makefile.in to allow unsigned int constants instead of C language standard signed int.

2.) include <sys/types.h> and avoid redefinition

3.) nonportable GCC specific ALIGN macro



Reproducible: Always

Actual Results:  
Compilation errors

Expected Results:  
successful compilation with standard conforming C compiler
Attached patch makes qcms compile on AIX (obsolete) — Splinter Review
The #ifdef should be optimized as the autoconfiguration build systems recognizes 
HAS_SYS_INTTYPES_H but this isn't honoured here.
The ALIGN macro is not portable to non-GCC compilers, I can append an -qalign=full  flag to CFLAGS in Makefile.in instead.
Error without patch:

gmake[1]: Entering directory `/home/ulink/Src/AIX-51/sm2/mozilla/gfx/qcms'
iccread.c
Building deps for /home/ulink/Src/comm-central/mozilla/gfx/qcms/iccread.c
xlc_r -qlanglvl=stdc99 -o iccread.o -c  -DMOZILLA_INTERNAL_API -DMOZ_SUITE=1 -DOSTYPE=\"AIX5.1\" -DOSARCH=AIX  -I/home/ulink/Src/comm-central/mozilla/gfx/qcms -I.  -I../../dist/include   -I../../dist/include/qcms -I/home/ulink/Src/AIX-51/sm2/mozilla/dist/include/nspr    -I/home/ulink/Src/AIX-51/sm2/mozilla/dist/sdk/include   -qflag=w:w     -DNDEBUG -DTRIMMED -O -DMOZ_QCMS -qenum=small   -DMOZILLA_VERSION=\"1.9.1.4\" -DMOZILLA_VERSION_U=1.9.1.4 -DAIX=1 -DHAVE_SYS_INTTYPES_H=1 -DNSCAP_DISABLE_DEBUG_PTR_TYPES=1 -DD_INO=d_ino -DSTDC_HEADERS=1 -DHAVE_ST_BLKSIZE=1 -DHAVE_SIGINFO_T=1 -DHAVE_INT16_T=1 -DHAVE_INT32_T=1 -DHAVE_INT64_T=1 -DHAVE_INT64=1 -DHAVE_UINT=1 -DHAVE_UINT_T=1 -DHAVE_UINT16_T=1 -DHAVE_DIRENT_H=1 -DHAVE_MEMORY_H=1 -DHAVE_UNISTD_H=1 -DHAVE_NL_TYPES_H=1 -DHAVE_MALLOC_H=1 -DHAVE_X11_XKBLIB_H=1 -DHAVE_SYS_STATVFS_H=1 -DHAVE_SYS_STATFS_H=1 -DHAVE_LIBC_R=1 -DHAVE_LIBM=1 -DHAVE_LIBDL=1 -DHAVE_LIBC_R=1 -DFUNCPROTO=15 -DHAVE_XSHM=1 -DHAVE_FT_BITMAP_SIZE_Y_PPEM=1 -DHAVE_FT_GLYPHSLOT_EMBOLDEN=1 -DHAVE_FT_LOAD_SFNT_TABLE=1 -DHAVE_ARM_SIMD=1 -D_REENTRANT=1 -DHAVE_RANDOM=1 -DHAVE_STRERROR=1 -DHAVE_LCHOWN=1 -DHAVE_FCHMOD=1 -DHAVE_SNPRINTF=1 -DHAVE_MEMMOVE=1 -DHAVE_RINT=1 -DHAVE_STAT64=1 -DHAVE_LSTAT64=1 -DHAVE_TRUNCATE64=1 -DHAVE_FLOCKFILE=1 -DHAVE_LOCALTIME_R=1 -DHAVE_STRTOK_R=1 -DHAVE_RES_NINIT=1 -DHAVE_LANGINFO_CODESET=1 -DHAVE_I18N_LC_MESSAGES=1 -DMOZ_EMBEDDING_LEVEL_DEFAULT=1 -DMOZ_EMBEDDING_LEVEL_BASIC=1 -DMOZ_EMBEDDING_LEVEL_MINIMAL=1 -DMOZ_BUILD_APP=../suite -DMOZ_XUL_APP=1 -DMOZ_DEFAULT_TOOLKIT=\"cairo-gtk2\" -DMOZ_X11=1 -DMOZ_WIDGET_GTK2=1 -DMOZ_ENABLE_XREMOTE=1 -DMOZ_DISTRIBUTION_ID=\"org.mozilla\" -DMOZ_PANGO=1 -DOJI=1 -DIBMBIDI=1 -DMOZ_VIEW_SOURCE=1 -DACCESSIBILITY=1 -DMOZ_XPINSTALL=1 -DMOZ_JSLOADER=1 -DNS_PRINTING=1 -DNS_PRINT_PREVIEW=1 -DMOZ_NO_XPCOM_OBSOLETE=1 -DMOZ_OGG=1 -DMOZ_WAVE=1 -DMOZ_SYDNEYAUDIO=1 -DMOZ_MEDIA=1 -DMOZ_XTF=1 -DMOZ_CRASHREPORTER_ENABLE_PERCENT=100 -DMOZ_MATHML=1 -DMOZ_ENABLE_CANVAS=1 -DMOZ_SVG=1 -DMOZ_UPDATE_CHANNEL=default -DMOZ_PLACES=1 -DMOZ_FEEDS=1 -DMOZ_STORAGE=1 -DMOZ_HELP_VIEWER=1 -DMOZ_LOGGING=1 -DMOZ_USER_DIR=\".mozilla\" -DHAVE_INTTYPES_H=1 -DMOZ_XUL=1 -DMOZ_PROFILELOCKING=1 -DMOZ_RDF=1 -DMOZ_MORK=1 -DMOZ_MORKREADER=1 -DMOZ_DLL_SUFFIX=\".so\" -DXP_UNIX=1 -DUNIX_ASYNC_DNS=1 -DMOZ_ACCESSIBILITY_ATK=1 -DATK_MAJOR_VERSION=1 -DATK_MINOR_VERSION=12 -DATK_REV_VERSION=3  -D_MOZILLA_CONFIG_H_ -DMOZILLA_CLIENT /home/ulink/Src/comm-central/mozilla/gfx/qcms/iccread.c
"/home/ulink/Src/comm-central/mozilla/gfx/qcms/qcmstypes.h", line 14.16: 1506-334 (S) Identifier int8_t has already been defined on line 65 of "/usr/include/sys/inttypes.h".
"/home/ulink/Src/comm-central/mozilla/gfx/qcms/qcmstypes.h", line 15.17: 1506-334 (S) Identifier uint8_t has already been defined on line 76 of "/usr/include/sys/inttypes.h".
"/home/ulink/Src/comm-central/mozilla/gfx/qcms/qcmstypes.h", line 16.17: 1506-334 (S) Identifier int16_t has already been defined on line 66 of "/usr/include/sys/inttypes.h".
"/home/ulink/Src/comm-central/mozilla/gfx/qcms/qcmstypes.h", line 17.18: 1506-334 (S) Identifier uint16_t has already been defined on line 77 of "/usr/include/sys/inttypes.h".
"/home/ulink/Src/comm-central/mozilla/gfx/qcms/qcmstypes.h", line 18.17: 1506-334 (S) Identifier int32_t has already been defined on line 67 of "/usr/include/sys/inttypes.h".
"/home/ulink/Src/comm-central/mozilla/gfx/qcms/qcmstypes.h", line 19.18: 1506-334 (S) Identifier uint32_t has already been defined on line 78 of "/usr/include/sys/inttypes.h".
"/home/ulink/Src/comm-central/mozilla/gfx/qcms/qcmstypes.h", line 20.17: 1506-334 (S) Identifier int64_t has already been defined on line 72 of "/usr/include/sys/inttypes.h".
"/home/ulink/Src/comm-central/mozilla/gfx/qcms/qcmstypes.h", line 21.18: 1506-334 (S) Identifier uint64_t has already been defined on line 83 of "/usr/include/sys/inttypes.h".
"/home/ulink/Src/comm-central/mozilla/gfx/qcms/qcmstypes.h", line 29.20: 1506-334 (S) Identifier uintptr_t has already been defined on line 105 of "/usr/include/sys/inttypes.h".
"/home/ulink/Src/comm-central/mozilla/gfx/qcms/qcmsint.h", line 63.27: 1506-195 (S) Integral constant expression with a value greater than zero is required.
"/home/ulink/Src/comm-central/mozilla/gfx/qcms/iccread.c", line 479.17: 1506-019 (S) Expecting an array or a pointer to object type.
"/home/ulink/Src/comm-central/mozilla/gfx/qcms/iccread.c", line 493.34: 1506-019 (S) Expecting an array or a pointer to object type.
gmake[1]: *** [iccread.o] Error 1
gmake[1]: Leaving directory `/home/ulink/Src/AIX-51/sm2/mozilla/gfx/qcms'
gmake: *** [default] Error 2
This patch does:
add -qenum flag in Makefile.in for non-standard enum types

include <sys/types.h> in qcmstypes.h

replace ALIGN macro in qcmsint.h and
replace nonstandard zero array iInt16Number data [0] in struct curveType
Attachment #407781 - Attachment is obsolete: true
Attachment #407982 - Flags: review?
Attachment #407982 - Flags: review? → review?(jmuizelaar)
Summary: typedef clashes on AIX with <sys/types.h> → qcms typedef clashes on AIX with <sys/types.h>
Attachment #407982 - Flags: review?(jmuizelaar)
Attachment #407982 - Attachment is obsolete: true
Comment on attachment 408348 [details] [diff] [review]
Revised patch with null macro ALIGN for AIX

Minimal patch for compiling with VisualAge / IBM XLC compiler
Attachment #408348 - Flags: review?(jmuizelaar)
Comment on attachment 408348 [details] [diff] [review]
Revised patch with null macro ALIGN for AIX

>diff -u8p mozilla-1.9.1-orig/gfx/qcms/Makefile.in mozilla-1.9.1/gfx/qcms/Makefile.in
>--- mozilla-1.9.1-orig/gfx/qcms/Makefile.in	2009-10-16 17:13:48.000000000 +0200
>+++ mozilla-1.9.1/gfx/qcms/Makefile.in	2009-10-26 10:03:37.000000000 +0100
>@@ -17,8 +17,12 @@ CSRCS = iccread.c transform.c
> 
> FORCE_STATIC_LIB = 1
> # This library is used by other shared libs
> FORCE_USE_PIC = 1
> 
> include $(topsrcdir)/config/rules.mk
> 
> CFLAGS          += -DMOZ_QCMS
>+
>+ifeq  ($(OS_ARCH),AIX)
>+CFLAGS += -qenum=small -qalign=full
>+endif

Can you add a comment about what these flags do and why they're needed?

>diff -u8p mozilla-1.9.1-orig/gfx/qcms/qcmsint.h mozilla-1.9.1/gfx/qcms/qcmsint.h
>--- mozilla-1.9.1-orig/gfx/qcms/qcmsint.h	2009-10-16 17:13:48.000000000 +0200
>+++ mozilla-1.9.1/gfx/qcms/qcmsint.h	2009-10-26 10:00:45.000000000 +0100
>@@ -7,16 +7,18 @@
> struct precache_output
> {
> 	int ref_count;
> 	uint8_t data[65535];
> };
> 
> #ifdef _MSC_VER
> #define ALIGN __declspec(align(16))
>+#elif defined AIX && __IBMC__
>+#define ALIGN /* empty macro for AIX */

How do we know that the array will be aligned? If you can't make any assurances about the alignment of matrix, you'll need to disable the sse/sse2 code. So the empty macro for ALIGN should be conditioned on not having the sse/sse2 code, and not on AIX/IBMC.

> #else
> #define ALIGN __attribute__(( aligned (16) ))
> #endif
> 
> struct _qcms_transform {
> 	float ALIGN matrix[3][4];
> 	float *input_gamma_table_r;
> 	float *input_gamma_table_g;
>@@ -55,17 +57,21 @@ typedef uint16_t uInt16Number;
> struct XYZNumber {
> 	s15Fixed16Number X;
> 	s15Fixed16Number Y;
> 	s15Fixed16Number Z;
> };
> 
> struct curveType {
> 	uint32_t count;
>+#ifdef AIX
>+	uInt16Number data[];
>+#else
> 	uInt16Number data[0];
>+#endif
> };
> 
> struct lutType {
> 	uint8_t num_input_channels;
> 	uint8_t num_output_channels;
> 	uint8_t num_clut_grid_points;
> 
> 	s15Fixed16Number e00;
>diff -u8p mozilla-1.9.1-orig/gfx/qcms/qcmstypes.h mozilla-1.9.1/gfx/qcms/qcmstypes.h
>--- mozilla-1.9.1-orig/gfx/qcms/qcmstypes.h	2009-10-16 17:13:48.000000000 +0200
>+++ mozilla-1.9.1/gfx/qcms/qcmstypes.h	2009-10-26 10:02:05.000000000 +0100
>@@ -5,16 +5,18 @@
> 
> #include "prtypes.h"
> 
> /* prtypes.h defines IS_LITTLE_ENDIAN and IS_BIG ENDIAN */
> 
> #if defined (__SVR4) && defined (__sun)
> /* int_types.h gets included somehow, so avoid redefining the types differently */
> #include <sys/int_types.h>
>+#elif defined (_AIX) 
>+#include <sys/types.h>
> #else
> typedef PRInt8 int8_t;
> typedef PRUint8 uint8_t;
> typedef PRInt16 int16_t;
> typedef PRUint16 uint16_t;
> typedef PRInt32 int32_t;
> typedef PRUint32 uint32_t;
> typedef PRInt64 int64_t;
>@@ -64,18 +66,16 @@ typedef unsigned __int32 uint32_t;
> typedef __int64 int64_t;
> typedef unsigned __int64 uint64_t;
> #ifdef _WIN64
> typedef unsigned __int64 uintptr_t;
> #else
> typedef unsigned long uintptr_t;
> #endif
> 

This change shouldn't be needed because it's not used when MOZ_QCMS is defined.

>-#elif defined (_AIX)
>-#  include <sys/inttypes.h>
> #else
> #  include <stdint.h>
> #endif
> 
> #endif
> 
> typedef qcms_bool bool;
> #define true 1
Attachment #408348 - Flags: review?(jmuizelaar) → review-
(In reply to comment #6)

> > include $(topsrcdir)/config/rules.mk
> > 
> > CFLAGS          += -DMOZ_QCMS
> >+
> >+ifeq  ($(OS_ARCH),AIX)
> >+CFLAGS += -qenum=small -qalign=full
> >+endif
> 
> Can you add a comment about what these flags do and why they're needed?

In qcms.h the is for e.g.
    icMaxEnumData                       = 0xFFFFFFFFL

member of an enum.

C standard defines signed int, so here: 0x7FFFFFFFL max. -qenum=small allows the compiler to choose the smallest type fitting. Less strict compilers like GCC or MSVC do this implicit w/o warning.

-qalign=full generates aligned Arrays instead of packed.

I cant put a comment in a revised patch.

> 
> >diff -u8p mozilla-1.9.1-orig/gfx/qcms/qcmsint.h mozilla-1.9.1/gfx/qcms/qcmsint.h
> >--- mozilla-1.9.1-orig/gfx/qcms/qcmsint.h	2009-10-16 17:13:48.000000000 +0200
> >+++ mozilla-1.9.1/gfx/qcms/qcmsint.h	2009-10-26 10:00:45.000000000 +0100
> >@@ -7,16 +7,18 @@
> > struct precache_output
> > {
> > 	int ref_count;
> > 	uint8_t data[65535];
> > };
> > 
> > #ifdef _MSC_VER
> > #define ALIGN __declspec(align(16))
> >+#elif defined AIX && __IBMC__
> >+#define ALIGN /* empty macro for AIX */
> 
> How do we know that the array will be aligned? If you can't make any assurances
> about the alignment of matrix, you'll need to disable the sse/sse2 code. So the
> empty macro for ALIGN should be conditioned on not having the sse/sse2 code,
> and not on AIX/IBMC.

AIX has no SSE/SSE2, without explicit compiler flags Altivec isn't used too.
AIX dropped support for the x86 plattform 15 Years ago. Else the -qalign flag ensures proper alignment for the PowerPC/POWER/rs6k platform.
If you can give me hint about macros identifying SSE/SSE2 I will rework the patch accordingly.
AIX won't compile Mozilla with GCC.

> > 
> > struct curveType {
> > 	uint32_t count;
> >+#ifdef AIX
> >+	uInt16Number data[];
> >+#else
> > 	uInt16Number data[0];
> >+#endif
> > };
> >diff -u8p mozilla-1.9.1-orig/gfx/qcms/qcmstypes.h mozilla-1.9.1/gfx/qcms/qcmstypes.h
> >--- mozilla-1.9.1-orig/gfx/qcms/qcmstypes.h	2009-10-16 17:13:48.000000000 +0200
> >+++ mozilla-1.9.1/gfx/qcms/qcmstypes.h	2009-10-26 10:02:05.000000000 +0100
> >@@ -5,16 +5,18 @@
> > 
> > #include "prtypes.h"
> > 
> > /* prtypes.h defines IS_LITTLE_ENDIAN and IS_BIG ENDIAN */
> > 
> > #if defined (__SVR4) && defined (__sun)
> > /* int_types.h gets included somehow, so avoid redefining the types differently */
> > #include <sys/int_types.h>
> >+#elif defined (_AIX) 
> >+#include <sys/types.h>
> > #else
> > typedef PRInt8 int8_t;
> 
> This change shouldn't be needed because it's not used when MOZ_QCMS is defined.

If not the compiler stops at the redefinition of types, therefore move the include <sys/inttypes.h> before and change to better style equivalent <sys/types.h> as <sys/inttypes.h> should not be included directly.

The next showstopper is the non C99 conforming flexible array data[0];
I don't wan't to change any semantics when porting to tier x platform AIX.
From my point of view the struct member data[]; should be okay for all without the #ifdef around. (???)

> 
> >-#elif defined (_AIX)
> >-#  include <sys/inttypes.h>
> > #else
> > #  include <stdint.h>
> > #endif
> > 
> > #endif
> > 
> > typedef qcms_bool bool;
> > #define true 1

this hunk is just a cleanup.
Bug 448176 is kindof duplicate for the meanwhile obsolete lcms.
(In reply to comment #7)
> (In reply to comment #6)
> 
> > > include $(topsrcdir)/config/rules.mk
> > > 
> > > CFLAGS          += -DMOZ_QCMS
> > >+
> > >+ifeq  ($(OS_ARCH),AIX)
> > >+CFLAGS += -qenum=small -qalign=full
> > >+endif
> > 
> > Can you add a comment about what these flags do and why they're needed?
> 
> In qcms.h the is for e.g.
>     icMaxEnumData                       = 0xFFFFFFFFL
> 
> member of an enum.
> 
> C standard defines signed int, so here: 0x7FFFFFFFL max. -qenum=small allows
> the compiler to choose the smallest type fitting. Less strict compilers like
> GCC or MSVC do this implicit w/o warning.
> 
> -qalign=full generates aligned Arrays instead of packed.
> 
> I cant put a comment in a revised patch.
> 
> > 
> > >diff -u8p mozilla-1.9.1-orig/gfx/qcms/qcmsint.h mozilla-1.9.1/gfx/qcms/qcmsint.h
> > >--- mozilla-1.9.1-orig/gfx/qcms/qcmsint.h	2009-10-16 17:13:48.000000000 +0200
> > >+++ mozilla-1.9.1/gfx/qcms/qcmsint.h	2009-10-26 10:00:45.000000000 +0100
> > >@@ -7,16 +7,18 @@
> > > struct precache_output
> > > {
> > > 	int ref_count;
> > > 	uint8_t data[65535];
> > > };
> > > 
> > > #ifdef _MSC_VER
> > > #define ALIGN __declspec(align(16))
> > >+#elif defined AIX && __IBMC__
> > >+#define ALIGN /* empty macro for AIX */
> > 
> > How do we know that the array will be aligned? If you can't make any assurances
> > about the alignment of matrix, you'll need to disable the sse/sse2 code. So the
> > empty macro for ALIGN should be conditioned on not having the sse/sse2 code,
> > and not on AIX/IBMC.
> 
> AIX has no SSE/SSE2, without explicit compiler flags Altivec isn't used too.
> AIX dropped support for the x86 plattform 15 Years ago. Else the -qalign flag
> ensures proper alignment for the PowerPC/POWER/rs6k platform.
> If you can give me hint about macros identifying SSE/SSE2 I will rework the
> patch accordingly.
> AIX won't compile Mozilla with GCC.

We should probably use a HAS_SSE define for this...

> If not the compiler stops at the redefinition of types, therefore move the
> include <sys/inttypes.h> before and change to better style equivalent
> <sys/types.h> as <sys/inttypes.h> should not be included directly.
> 

I was referring to the hunk below...

> The next showstopper is the non C99 conforming flexible array data[0];
> I don't wan't to change any semantics when porting to tier x platform AIX.
> From my point of view the struct member data[]; should be okay for all without
> the #ifdef around. (???)
> 

Let me think about this one...

> > 
> > >-#elif defined (_AIX)
> > >-#  include <sys/inttypes.h>
> > > #else
> > > #  include <stdint.h>
> > > #endif
> > > 
> > > #endif
> > > 
> > > typedef qcms_bool bool;
> > > #define true 1
> 
> this hunk is just a cleanup.

Doesn't removing it break compilation when MOZ_QCMS is not defined?
You're right. The problem did not appear because MOZ_QCMS was set automatically.
I will change <sys/inttypes.h> to <sys/types.h> which will include <sys/inttypes.h> and <standards.h>.
Else nothing bad will ever happen as all system headers are properly ifdef against multiple inclusion. AIX compiler is *really* picky about type redefinition.

Correct me if I'm wrong: MOZ_QCMS is not set if configuring using system lcms?
in transform.c I find __SSE2_ && __GNUC__

so it should be like

 #ifdef _MSC_VER
 #define ALIGN __declspec(align(16))
 #elif not defined __SSE2__ && __GNUC__
 #define ALIGN /* RISC Platforms not using GNU compiler */
 #else
 #define ALIGN __attribute__(( aligned (16) )) /* GCC */
 #endif

or
 
 #ifdef _MSC_VER
 #define ALIGN __declspec(align(16))
 #elif defined __SSE2__ && __GNUC__ 
 #define ALIGN __attribute__(( aligned (16) )) /* GCC */
 #else
 #define  ALIGN /* RISC Platforms not using GNU compiler */
 #endif

What about Solaris/Sparc and HP-UX/PA-RISC ?
Attached patch New patch (obsolete) — Splinter Review
ULi:

   Please correct me if I am wrong. With the compiler version 8 and compiler version 9, the issue with ALLIGN macro does not exist.

Hence I modified the patch and attached the same.
Attachment #411138 - Flags: review?(jmuizelaar)
Can you please review the new attachment ?
Comment on attachment 411138 [details] [diff] [review]
New patch

> struct curveType {
> 	uint32_t count;
>+#ifdef AIX
>+        uInt16Number data[];
>+#else
> 	uInt16Number data[0];
>+#endif
> };

The indentation is off for the added lines. The ifdef should probably use __IBMC__
instead of AIX because it's the IBM compiler that doesn't support the zero-length array gcc/msvc extension. Adding a comment about using the C99 flexible array member syntax with IBM compiler would be nice to have too :)

Other than that it looks good.
Attachment #411138 - Flags: review?(jmuizelaar) → review+
"uInt16Number date[];" 
is mandatory C99 standard, but GCC's C99 interpretation is more conforming to GCC's legacy than the ISO standards ;-)
Attached patch Patch V 3 (obsolete) — Splinter Review
All review comments are taken care of with this patch.
Attachment #411138 - Attachment is obsolete: true
Attachment #418344 - Flags: review?(jmuizelaar)
Attached patch Patch V 4Splinter Review
Taken care of indentation
Attachment #418344 - Attachment is obsolete: true
Attachment #418345 - Flags: review?(jmuizelaar)
Attachment #418344 - Flags: review?(jmuizelaar)
Attachment #418345 - Flags: review?(jmuizelaar) → review+
Attachment #418345 - Flags: superreview?(roc)
Attachment #418345 - Flags: superreview?(roc) → superreview+
Keywords: checkin-needed
Assignee: nobody → shailen.n.jain
Attachment #408348 - Attachment is obsolete: true
The patch doesn't apply cleanly for me.
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
(In reply to comment #18)
> The patch doesn't apply cleanly for me.

It has DOS line-endings for one, they should be UNIX-ified...
http://hg.mozilla.org/mozilla-central/rev/a2219367e0cf
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
The change has not been put to mozilla-1.9.2 branch
You need to log in before you can comment on or make changes to this bug.