You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@openoffice.apache.org by Pavel Janík <Pa...@Janik.cz> on 2012/08/14 15:47:46 UTC
Re: svn commit: r1372411 - /incubator/ooo/trunk/main/sw/source/filter/ww8/ww8par5.cxx
Hi,
On Aug 13, 2012, at 3:04 PM, orw@apache.org wrote:
> Author: orw
> Date: Mon Aug 13 13:04:34 2012
> New Revision: 1372411
>
> URL: http://svn.apache.org/viewvc?rev=1372411&view=rev
> Log:
> #119440# - method <SwWW8ImplReader::Read_SubF_Combined(..)>
> -- add support for further field codes
>
> Found by: Yan Ji
> Patch by: zjcen
> Review by: orw
>
> Modified:
> incubator/ooo/trunk/main/sw/source/filter/ww8/ww8par5.cxx
>
> Modified: incubator/ooo/trunk/main/sw/source/filter/ww8/ww8par5.cxx
> URL: http://svn.apache.org/viewvc/incubator/ooo/trunk/main/sw/source/filter/ww8/ww8par5.cxx?rev=1372411&r1=1372410&r2=1372411&view=diff
> ==============================================================================
> --- incubator/ooo/trunk/main/sw/source/filter/ww8/ww8par5.cxx (original)
> +++ incubator/ooo/trunk/main/sw/source/filter/ww8/ww8par5.cxx Mon Aug 13 13:04:34 2012
> + const sal_Unicode cC = sPart.GetChar(nBegin+1);
> + if ( (-1 < cC) && (cC < 32) )
This code results in this warning:
sw/source/filter/ww8/ww8par5.cxx:2588: warning: comparison is always true due to limited range of data type
Please fix this.
--
Pavel Janík
Re: svn commit: r1372411 - /incubator/ooo/trunk/main/sw/source/filter/ww8/ww8par5.cxx
Posted by ZuoJun Chen <zj...@gmail.com>.
Hi,
I use char type for variable cC before and it should be signed as
default in GCC
as far as I remember. but we can modify that with -funsigned-char, so it is
reasonable
to use sal_Unicode when we handle Unicode string. The comparison is
previously
used with char, and I have no change the condition properly after
replacement.
Thanks for your careful check and explanation.
Regards - Zuojun
2012/8/14 Oliver-Rainer Wittmann <or...@googlemail.com>
> Hi Pavel,
>
>
> On 14.08.2012 16:27, Pavel Janík wrote:
>
>> Hi Oliver,
>>
>> On Aug 14, 2012, at 4:15 PM, Oliver-Rainer Wittmann wrote:
>>
>> --- incubator/ooo/trunk/main/sw/**source/filter/ww8/ww8par5.cxx
>>>>> (original)
>>>>> +++ incubator/ooo/trunk/main/sw/**source/filter/ww8/ww8par5.cxx Mon
>>>>> Aug 13 13:04:34 2012
>>>>>
>>>>
>>>> + const sal_Unicode cC =
>>>>> sPart.GetChar(nBegin+1);
>>>>> + if ( (-1 < cC) && (cC < 32) )
>>>>>
>>>>
>>>> This code results in this warning:
>>>>
>>>> sw/source/filter/ww8/ww8par5.**cxx:2588: warning: comparison is always
>>>> true due to limited range of data type
>>>>
>>>> Please fix this.
>>>>
>>>>
>>> Hm.
>>>
>>> This the first sample document attached to issue 119440 I am observing
>>> that this comparison is not always true. I debugged the this code during my
>>> patch review.
>>> The string <sPart> contains data from an imported Microsoft Word
>>> document which is not a fix.
>>>
>>> May be I have overseen something.
>>> Pavel, can you provide further information why in your environment this
>>> comparison is always true?
>>>
>>
>>
>> I think that
>>
>> Pavel-Janiks-MacBook-Pro:ooo_**trunk_src pavel$ grep -r sal_Unicode sal
>> | grep typedef
>> ...
>> sal/inc/sal/types.h: typedef sal_uInt16 sal_Unicode;
>> ...
>> Pavel-Janiks-MacBook-Pro:ooo_**trunk_src pavel$
>>
>> and thus sal_Unicode is unsigned here...
>>
>> Maybe on Windows it is not:
>>
>> sal/inc/sal/types.h: typedef wchar_t sal_Unicode;
>>
>>
> Now, I got it.
> I thought that you meant that the complete camparison condition is
> reported to be always true.
> I will take of this.
>
> Thanks for opening my eyes.
>
> Best regards, Oliver.
>
Re: svn commit: r1372411 - /incubator/ooo/trunk/main/sw/source/filter/ww8/ww8par5.cxx
Posted by Oliver-Rainer Wittmann <or...@googlemail.com>.
Hi Pavel,
On 14.08.2012 16:27, Pavel Janík wrote:
> Hi Oliver,
>
> On Aug 14, 2012, at 4:15 PM, Oliver-Rainer Wittmann wrote:
>
>>>> --- incubator/ooo/trunk/main/sw/source/filter/ww8/ww8par5.cxx (original)
>>>> +++ incubator/ooo/trunk/main/sw/source/filter/ww8/ww8par5.cxx Mon Aug 13 13:04:34 2012
>>>
>>>> + const sal_Unicode cC = sPart.GetChar(nBegin+1);
>>>> + if ( (-1 < cC) && (cC < 32) )
>>>
>>> This code results in this warning:
>>>
>>> sw/source/filter/ww8/ww8par5.cxx:2588: warning: comparison is always true due to limited range of data type
>>>
>>> Please fix this.
>>>
>>
>> Hm.
>>
>> This the first sample document attached to issue 119440 I am observing that this comparison is not always true. I debugged the this code during my patch review.
>> The string <sPart> contains data from an imported Microsoft Word document which is not a fix.
>>
>> May be I have overseen something.
>> Pavel, can you provide further information why in your environment this comparison is always true?
>
>
> I think that
>
> Pavel-Janiks-MacBook-Pro:ooo_trunk_src pavel$ grep -r sal_Unicode sal | grep typedef
> ...
> sal/inc/sal/types.h: typedef sal_uInt16 sal_Unicode;
> ...
> Pavel-Janiks-MacBook-Pro:ooo_trunk_src pavel$
>
> and thus sal_Unicode is unsigned here...
>
> Maybe on Windows it is not:
>
> sal/inc/sal/types.h: typedef wchar_t sal_Unicode;
>
Now, I got it.
I thought that you meant that the complete camparison condition is reported to
be always true.
I will take of this.
Thanks for opening my eyes.
Best regards, Oliver.
Re: svn commit: r1372411 - /incubator/ooo/trunk/main/sw/source/filter/ww8/ww8par5.cxx
Posted by Pavel Janík <Pa...@Janik.cz>.
Hi Oliver,
On Aug 14, 2012, at 4:15 PM, Oliver-Rainer Wittmann wrote:
>>> --- incubator/ooo/trunk/main/sw/source/filter/ww8/ww8par5.cxx (original)
>>> +++ incubator/ooo/trunk/main/sw/source/filter/ww8/ww8par5.cxx Mon Aug 13 13:04:34 2012
>>
>>> + const sal_Unicode cC = sPart.GetChar(nBegin+1);
>>> + if ( (-1 < cC) && (cC < 32) )
>>
>> This code results in this warning:
>>
>> sw/source/filter/ww8/ww8par5.cxx:2588: warning: comparison is always true due to limited range of data type
>>
>> Please fix this.
>>
>
> Hm.
>
> This the first sample document attached to issue 119440 I am observing that this comparison is not always true. I debugged the this code during my patch review.
> The string <sPart> contains data from an imported Microsoft Word document which is not a fix.
>
> May be I have overseen something.
> Pavel, can you provide further information why in your environment this comparison is always true?
I think that
Pavel-Janiks-MacBook-Pro:ooo_trunk_src pavel$ grep -r sal_Unicode sal | grep typedef
...
sal/inc/sal/types.h: typedef sal_uInt16 sal_Unicode;
...
Pavel-Janiks-MacBook-Pro:ooo_trunk_src pavel$
and thus sal_Unicode is unsigned here...
Maybe on Windows it is not:
sal/inc/sal/types.h: typedef wchar_t sal_Unicode;
--
Pavel Janík
Re: svn commit: r1372411 - /incubator/ooo/trunk/main/sw/source/filter/ww8/ww8par5.cxx
Posted by Oliver-Rainer Wittmann <or...@googlemail.com>.
Hi,
On 14.08.2012 15:47, Pavel Janík wrote:
> Hi,
>
> On Aug 13, 2012, at 3:04 PM, orw@apache.org wrote:
>
>> Author: orw
>> Date: Mon Aug 13 13:04:34 2012
>> New Revision: 1372411
>>
>> URL: http://svn.apache.org/viewvc?rev=1372411&view=rev
>> Log:
>> #119440# - method <SwWW8ImplReader::Read_SubF_Combined(..)>
>> -- add support for further field codes
>>
>> Found by: Yan Ji
>> Patch by: zjcen
>> Review by: orw
>>
>> Modified:
>> incubator/ooo/trunk/main/sw/source/filter/ww8/ww8par5.cxx
>>
>> Modified: incubator/ooo/trunk/main/sw/source/filter/ww8/ww8par5.cxx
>> URL: http://svn.apache.org/viewvc/incubator/ooo/trunk/main/sw/source/filter/ww8/ww8par5.cxx?rev=1372411&r1=1372410&r2=1372411&view=diff
>> ==============================================================================
>> --- incubator/ooo/trunk/main/sw/source/filter/ww8/ww8par5.cxx (original)
>> +++ incubator/ooo/trunk/main/sw/source/filter/ww8/ww8par5.cxx Mon Aug 13 13:04:34 2012
>
>> + const sal_Unicode cC = sPart.GetChar(nBegin+1);
>> + if ( (-1 < cC) && (cC < 32) )
>
> This code results in this warning:
>
> sw/source/filter/ww8/ww8par5.cxx:2588: warning: comparison is always true due to limited range of data type
>
> Please fix this.
>
Hm.
This the first sample document attached to issue 119440 I am observing that this
comparison is not always true. I debugged the this code during my patch review.
The string <sPart> contains data from an imported Microsoft Word document which
is not a fix.
May be I have overseen something.
Pavel, can you provide further information why in your environment this
comparison is always true?
Best regards, Oliver.