Re: [Libreoffice] [PATCH] SAL_N_ELEMENTS for writer, impress and calc

On Sat, Oct 9, 2010 at 11:03 AM, Kayo Hamid <revol.code@yahoo.com> wrote:

> I'm sending my patchs for review, can anyone see and reply back if it is ok
> or not? I do not known if it is correct ;-)
>
> revol_

Hi revol_

Thanks for the patch.

One remark. You want to use the macro SAL_N_ELEMENTS, not define your own
ELEMENTS_OF_ARRAY in every sources.

for example
--- a/starmath/source/cfgitem.cxx
+++ b/starmath/source/cfgitem.cxx
@@ -47,6 +47,7 @@ static const char* aRootName = "Office.Math";

 #define SYMBOL_LIST         "SymbolList"
 #define FONT_FORMAT_LIST    "FontFormatList"
+#define ELEMENTS_OF_ARRAY(arr) (sizeof(arr)/(sizeof((arr)[0])))

 SV_IMPL_OBJARR( SmFntFmtListEntryArr, SmFntFmtListEntry );

@@ -184,14 +185,14 @@ static Sequence< OUString > lcl_GetPropertyNames(

 static Sequence< OUString > GetFormatPropertyNames()
 {
-    USHORT nCnt = sizeof(aFormatPropNames) / sizeof(aFormatPropNames[0]);
+    USHORT nCnt = ELEMENTS_OF_ARRAY(aFormatPropNames);
     return lcl_GetPropertyNames( aFormatPropNames, nCnt );
 }


 static Sequence< OUString > GetOtherPropertyNames()
 {
-    USHORT nCnt = sizeof(aMathPropNames) / sizeof(aMathPropNames[0]);
+    USHORT nCnt = ELEMENTS_OF_ARRAY(aMathPropNames);
     return lcl_GetPropertyNames( aMathPropNames, nCnt );
 }

should read something like


--- a/starmath/source/cfgitem.cxx
+++ b/starmath/source/cfgitem.cxx
@@ -184,14 +185,14 @@ static Sequence< OUString > lcl_GetPropertyNames(

 static Sequence< OUString > GetFormatPropertyNames()
 {
-    USHORT nCnt = sizeof(aFormatPropNames) / sizeof(aFormatPropNames[0]);
+    USHORT nCnt = SAL_N_ELEMENTS(aFormatPropNames);
     return lcl_GetPropertyNames( aFormatPropNames, nCnt );
 }


 static Sequence< OUString > GetOtherPropertyNames()
 {
-    USHORT nCnt = sizeof(aMathPropNames) / sizeof(aMathPropNames[0]);
+    USHORT nCnt = SAL_N_ELEMENTS(aMathPropNames);
     return lcl_GetPropertyNames( aMathPropNames, nCnt );
 }

Other than that, it looks good.

Norbert



> _______________________________________________
> LibreOffice mailing list
> LibreOffice@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/libreoffice