-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105519/#review15973
-----------------------------------------------------------


I wonder if it would no be a better fix to add a parameter for the default 
value to the macro STRING_TO_INT, to which the variable gets set if the string 
is empty. Might be more readable code not only to me.

Only issue would be the usage in
        STRING_TO_INT(pitchFamily, pitchFamilyInt, "latin@pitchFamily")
as that needs no default value, given that "if (!pitchFamily.isEmpty()) {" 
already before. But then this code is already now not a perfect match for the 
macro, as the macro again checks if pitchFamily is empty.
Perhaps that code could be changed from
    if (!pitchFamily.isEmpty()) {
        int pitchFamilyInt;
        STRING_TO_INT(pitchFamily, pitchFamilyInt, "latin@pitchFamily")
to
    int pitchFamilyInt;
    STRING_TO_INT(pitchFamily, pitchFamilyInt, -1, "latin@pitchFamily")
    if (pitchFamilyInt != -1) {
with the new macro.

Just a proposal. Because you will find out that this patch is not complete, 
there are more places where the original variable will get used uninitliazed, 
because there is no default value. With the same macro STRING_TO_INT this seems 
at least true for
int m_svgWidth; //! set by read_ext()
int m_svgHeight; //! set by read_ext()
int m_svgChX; //!< set by read_chOff()
int m_svgChY; //!< set by read_chOff()
int m_svgChWidth; //! set by read_chExt()
int m_svgChHeight; //! set by read_chExt()
All are never initialized if their string is empty. And as the other 
STRING_TO_* macros seem to follow the same pattern, there is even more to do :)


- Friedrich W. H. Kossebau


On July 11, 2012, 9:43 p.m., Jarosław Staniek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105519/
> -----------------------------------------------------------
> 
> (Updated July 11, 2012, 9:43 p.m.)
> 
> 
> Review request for Calligra and Matus Uzak.
> 
> 
> Description
> -------
> 
> Fixed '<value> may be used uninitialized' warnings
> 
> In member function KoFilter::ConversionStatus 
> XlsxXmlWorksheetReader::read_spcPct(): lineSpace may be used uninitialized in 
> this function
> In member function KoFilter::ConversionStatus 
> XlsxXmlWorksheetReader::read_spcPts(): margin may be used uninitialized in 
> this function
> 
> 
> Diffs
> -----
> 
>   filters/libmsooxml/MsooXmlCommonReaderDrawingMLImpl.h 
> 492f2ea7f6f01a0893d3f7a26b7cc2b36e82d49e 
> 
> Diff: http://git.reviewboard.kde.org/r/105519/diff/
> 
> 
> Testing
> -------
> 
>  
> 
> 
> Thanks,
> 
> Jarosław Staniek
> 
>

_______________________________________________
calligra-devel mailing list
calligra-devel@kde.org
https://mail.kde.org/mailman/listinfo/calligra-devel

Reply via email to