Re: svx/source/svdraw/svdfmtf.cxx
On 20.05.2012 13:30, Armin Le Grand wrote:
Hi Pavel,
Pavel Janíkpa...@janik.cz wrote:
Hi,
WaE = Warning as Error.
or Warnings Are Errors.
Gcc option -Werror: Make all warnings into errors.
With this option turned on, all warnings are made into errors. Our long
term goal is to make gcc silent.
Ah, yes, I remember now. We spent some time on this years ago, but never
coud make the whole code WaE-safe.
I thought that most of non-binfilter code was WaE-safe.
Anyway, we should still try to keep/make our code WaE-safe. It can only
improve its quality.
Despite that, it's good to work on it;
sometimes this gives good hints at weird code.
These issues are not errors per se, but e.g.:
@@ -1330,6 +1331,7 @@
void ImpSdrGDIMetaFileImport::DoAction(MetaWallpaperAction rAct)
Please just comment /*rAct*/
{
+(void) rAct;
OSL_ENSURE(false, Tried to construct SdrObject from
MetaWallpaperAction: not supported (!));
}
This means that rAct is unused in the method. gcc warns about it.
This change:
@@ -1384,6 +1388,7 @@
case GRADIENT_ELLIPTICAL: aXGradientStyle = XGRAD_ELLIPTICAL;
break;
case GRADIENT_SQUARE: aXGradientStyle = XGRAD_SQUARE; break;
case GRADIENT_RECT: aXGradientStyle = XGRAD_RECT; break;
+ default: break;
Hmm. I do not have the code at hand right now, cannot tell until monday.
}
const XFillGradientItem aXFillGradientItem(
means that some enum value is forgotten in the switch.
This change:
-for(sal_uInt32 y(0); y pOld-Height(); y++)
+for(sal_Int32 y(0); y pOld-Height(); y++)
Please change to sal_uInt32
Why? Does not look like y could become negative (assuming that
pOld-Height() is also a sal_uInt32.
means that we were comparing signed and unsigned value.
I do not know if these changes are OK, thus I send the patch as I used to
make the module WaE free.
I have not seen a patch. If you have one, please send again (maybe
directly) and I'll happily take a look on monday. I'm currently not
compiling on gcc, so I will not be able to guarantee, though.
Hope this helps.
P.S. Of course warnings differ between compilers and sometims the changes look
weird etc.
Yes, I remember now. Do you have a good solution for swich..case where not
all missing entries would have to be listed?
Pavel has already done that above (add default: break; to your switch
statement.
Who will check in the changes?
-Andre
Re: svx/source/svdraw/svdfmtf.cxx
Hi, On 21.05.2012 11:34, Andre Fischer wrote: On 20.05.2012 13:30, Armin Le Grand wrote: Hi Pavel, [..] Who will check in the changes? Already did, Pavel is informed. -Andre Sincerely, Armin -- ALG
Re: svx/source/svdraw/svdfmtf.cxx
On May 21, 2012, at 2:24 PM, Armin Le Grand wrote: Who will check in the changes? Already did, Pavel is informed. svx module is WaE clean now. Will continue with cui and chart2 now. -- Pavel Janík
Re: svx/source/svdraw/svdfmtf.cxx
On 21.05.2012 11:34, Andre Fischer wrote: [...] I thought that most of non-binfilter code was WaE-safe. The code base was WaE-clean only for certain compiler versions. IIRC gcc4.2 on Linux, gcc4.0 on OSX and VC2008 on Win. But improved (or any changed) compilers often provide new warnings because they understand aspects of the code better. Anyway, we should still try to keep/make our code WaE-safe. It can only improve its quality. +1, but we should avoid diminishing the code's readability too much by it. Herbert
Re: svx/source/svdraw/svdfmtf.cxx
Anyway, we should still try to keep/make our code WaE-safe. It can only improve its quality. +1, but we should avoid diminishing the code's readability too much by it. Yes. This is the reason why I'd like to see lines like default: break; commented ;-)) -- Pavel Janík
Re: svx/source/svdraw/svdfmtf.cxx
Hi Pavel,
Pavel Janík pa...@janik.cz wrote:
Hi,
WaE = Warning as Error.
or Warnings Are Errors.
Gcc option -Werror: Make all warnings into errors.
With this option turned on, all warnings are made into errors. Our long
term goal is to make gcc silent.
Ah, yes, I remember now. We spent some time on this years ago, but never
coud make the whole code WaE-safe. Despite that, it's good to work on it;
sometimes this gives good hints at weird code.
These issues are not errors per se, but e.g.:
@@ -1330,6 +1331,7 @@
void ImpSdrGDIMetaFileImport::DoAction(MetaWallpaperAction rAct)
Please just comment /*rAct*/
{
+(void) rAct;
OSL_ENSURE(false, Tried to construct SdrObject from
MetaWallpaperAction: not supported (!));
}
This means that rAct is unused in the method. gcc warns about it.
This change:
@@ -1384,6 +1388,7 @@
case GRADIENT_ELLIPTICAL: aXGradientStyle =
XGRAD_ELLIPTICAL; break;
case GRADIENT_SQUARE: aXGradientStyle = XGRAD_SQUARE; break;
case GRADIENT_RECT: aXGradientStyle = XGRAD_RECT; break;
+ default: break;
Hmm. I do not have the code at hand right now, cannot tell until monday.
}
const XFillGradientItem aXFillGradientItem(
means that some enum value is forgotten in the switch.
This change:
-for(sal_uInt32 y(0); y pOld-Height(); y++)
+for(sal_Int32 y(0); y pOld-Height(); y++)
Please change to sal_uInt32
means that we were comparing signed and unsigned value.
I do not know if these changes are OK, thus I send the patch as I used to
make the module WaE free.
I have not seen a patch. If you have one, please send again (maybe
directly) and I'll happily take a look on monday. I'm currently not
compiling on gcc, so I will not be able to guarantee, though.
Hope this helps.
P.S. Of course warnings differ between compilers and sometims the changes
look weird etc.
Yes, I remember now. Do you have a good solution for swich..case where not
all missing entries would have to be listed?
--
ALG
Re: svx/source/svdraw/svdfmtf.cxx
Hi Pavel, I worked on it. What do you mean with WaE? I googled, but couild not find explanations. I know there was something, but I do not want to rely on guessing. Pavel Janík pa...@janik.cz wrote: Hi, this file in the trunk has several WaE issues: http://tmp.janik.cz/AOOo/svx-WaE.diff -- ALG
Re: svx/source/svdraw/svdfmtf.cxx
Am 05/19/2012 01:22 PM, schrieb Armin Le Grand: Hi Pavel, I worked on it. What do you mean with WaE? I googled, but couild not find explanations. I know there was something, but I do not want to rely on guessing. WaE = Warning as Error. HTH Marcus Pavel Janíkpa...@janik.cz wrote: Hi, this file in the trunk has several WaE issues: http://tmp.janik.cz/AOOo/svx-WaE.diff
Re: svx/source/svdraw/svdfmtf.cxx
Marcus (OOo) marcus.m...@wtnet.de wrote: Am 05/19/2012 01:22 PM, schrieb Armin Le Grand: Hi Pavel, I worked on it. What do you mean with WaE? I googled, but couild not find explanations. I know there was something, but I do not want to rely on guessing. WaE = Warning as Error. Ah, okay, thanks. So, what aspect of that file should be different? It wants to warn for non-impemented actions, but it does not need to be an error. If the result looks weird, the issued warnings in a debug version may help as a hint for what went wrong. What should be different? HTH Marcus Pavel Janíkpa...@janik.cz wrote: Hi, this file in the trunk has several WaE issues: http://tmp.janik.cz/AOOo/svx-WaE.diff -- ALG
Re: svx/source/svdraw/svdfmtf.cxx
Hi,
WaE = Warning as Error.
or Warnings Are Errors.
Gcc option -Werror: Make all warnings into errors.
With this option turned on, all warnings are made into errors. Our long term
goal is to make gcc silent.
These issues are not errors per se, but e.g.:
@@ -1330,6 +1331,7 @@
void ImpSdrGDIMetaFileImport::DoAction(MetaWallpaperAction rAct)
{
+(void) rAct;
OSL_ENSURE(false, Tried to construct SdrObject from MetaWallpaperAction:
not supported (!));
}
This means that rAct is unused in the method. gcc warns about it.
This change:
@@ -1384,6 +1388,7 @@
case GRADIENT_ELLIPTICAL: aXGradientStyle = XGRAD_ELLIPTICAL;
break;
case GRADIENT_SQUARE: aXGradientStyle = XGRAD_SQUARE; break;
case GRADIENT_RECT: aXGradientStyle = XGRAD_RECT; break;
+ default: break;
}
const XFillGradientItem aXFillGradientItem(
means that some enum value is forgotten in the switch.
This change:
-for(sal_uInt32 y(0); y pOld-Height(); y++)
+for(sal_Int32 y(0); y pOld-Height(); y++)
means that we were comparing signed and unsigned value.
I do not know if these changes are OK, thus I send the patch as I used to make
the module WaE free.
Hope this helps.
P.S. Of course warnings differ between compilers and sometims the changes look
weird etc.
--
Pavel Janík
