Re: comctl32/tests: Removed sign comparison warning in datetime tests.
Joris Huizer wrote:
> On 10/05/2011 04:43 PM, Marko Nikolic wrote:
>> static void test_mccolor_types(HWND hWndDateTime, int mccolor_type, const
>> char* mccolor_name)
>> {
>> -LRESULT r;
>> -COLORREF theColor, prevColor;
>> +//LRESULT r;
>> +COLORREF theColor, prevColor, crColor;
>>
>
> Please avoid C++/C99-style comments in Wine code
> In this case you should remove the 'r' variable instead of commenting it
> out.
Thanks, I missed it, it left from the testing. The whole line should be
removed, I'll resend the patch.
Marko
Re: comctl32/tests: Using unsigned constants to remove sign comparison warning.
David Laight wrote: > On Fri, May 27, 2011 at 10:09:04PM +0400, Nikolay Sivov wrote: >> On Fri, May 27, 2011 at 5:16 PM, Marko Nikolic wrote: >> > --- >> > ?dlls/comctl32/tests/treeview.c | ? ?6 +++--- >> > ?1 files changed, 3 insertions(+), 3 deletions(-) >> > >> > diff --git a/dlls/comctl32/tests/treeview.c >> > b/dlls/comctl32/tests/treeview.c index 5fd26a0..b1eb85e 100644 >> > --- a/dlls/comctl32/tests/treeview.c >> > +++ b/dlls/comctl32/tests/treeview.c >> > @@ -634,7 +634,7 @@ static void test_get_set_bkcolor(void) >> > >> > ? ? /* If the value is -1, the control is using the system color for >> > the background color. */ ? ? crColor = (COLORREF)SendMessage( hTree, >> > TVM_GETBKCOLOR, 0, 0 ); - ? ?ok(crColor == -1, "Default background >> > color reported as 0x%.8x\n", crColor); + ? ?ok(crColor == 0x, >> > "Default background color reported as 0x%.8x\n", crColor); >> > >> >> ~0 looks better here imo. > > Rather ~0u > The 0x should be 0xu > > David > You are right, thanks, I'll update the patch. Marko
Re: cabinet/tests: Reformated output in case of test failure (try 2)
Alexandre Julliard wrote: > Marko Nikolic writes: > >> In case of test failure, expected and received values are traced. >> Format for the two values was different, corrected so both values >> are traced as hex. > > Error values are in general more useful in decimal. > You are right, I have removed format change for all error values.
Re: [PATCH] shell32: use flexible arrays to avoid fortify failures
Henri Verbeet wrote: > On 14 September 2010 15:44, Mike Frysinger wrote: >> note: i couldnt find a statement of what C standard wine aims for. if >> it is attempting pre-c99, then this will have to be done differently. >> perhaps introducing a project-wide define like "VARARRAY" which >> expands into [] for c99+ and [1] for older ... >> > Roughly C89, C99 features are generally out. Hi, In the wine wiki it is stated that "Wine adheres to standard C89/C90." http://wiki.winehq.org/SubmittingPatches#head-7a578af49f1d1ab7f36f4b1f98b7544bb55eea9a Marko
Re: avifil32: Removed sign comparison warning (sizeof expresions)
Andrey Turkin wrote: > On Monday 23 August 2010 13:16:07 Michael Stefaniuc wrote: >> IMHO gcc is *wrong* in emitting a warning there. sizeof(PCMWAVEFORMAT) >> is a compile time constant and gcc can see that sizeof(PCMWAVEFORMAT) >> falls well inside the number range expressible by a LONG. Logically >> there is no difference between >> formatsize <= sizeof(PCMWAVEFORMAT) >> and >> formatsize <= 16 >> One gives a bogus warning and the other doesn't. > > C99 std (para 6.5.3.4.4) states following about sizeof operator: > "...its type (an unsigned integer type) is size_t, deļ¬ned in > (and other headers)." > > sizeof result is a compile-time constant but, unlike numeric constants, > its type must always be size_t so gcc does the correct thing here. The same is with C89, paragraph 3.3.3.4 from standard: The value of the result is implementation-defined, and its type (an unsigned integral type) is size_t defined in the header. So, the gcc is correct. In the example above, correct would be formatsize <= 16U which gives a warning, as expected. What remains is how to correctly remove warning. In this case (and there are many similar in the code), signed function parameter is comparing with values that are natively unsigned. Changing type of the parameter is not possible, the same if with sizeof operator. One possiblity is to add some temporary variable, but in my opinioin it will just unncesary bloat the code and is worse than casting return value of sizeof. Maybe better solution is to make signed_sizeof macro, which will always return signed values and use it instead of sizeof; I will check if there is possiblity for that.
Re: appwiz.cpl: Removed sign comparison warning
Nikolay Sivov wrote:
> On 8/21/2010 12:18, Marko Nikolic wrote:
>> Nikolay Sivov wrote:
>>
>>>On 8/20/2010 20:04, Marko Nikolic wrote:
>>>> Changed variable type to match function return type.
>>>> ---
>>>>dlls/appwiz.cpl/appwiz.c |2 +-
>>>>1 files changed, 1 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/dlls/appwiz.cpl/appwiz.c b/dlls/appwiz.cpl/appwiz.c
>>>> index ffd2b24..1b3370b 100644
>>>> --- a/dlls/appwiz.cpl/appwiz.c
>>>> +++ b/dlls/appwiz.cpl/appwiz.c
>>>> @@ -406,7 +406,7 @@ static void UpdateButtons(HWND hWnd)
>>>>{
>>>>APPINFO *iter;
>>>>LVITEMW lvItem;
>>>> -DWORD selitem = SendDlgItemMessageW(hWnd, IDL_PROGRAMS,
>>>> LVM_GETNEXTITEM, -1,
>>>> +LRESULT selitem = SendDlgItemMessageW(hWnd, IDL_PROGRAMS,
>>>> LVM_GETNEXTITEM, -1,
>>>> LVNI_FOCUSED | LVNI_SELECTED);
>>>>BOOL enable_modify = FALSE;
>>>>
>>> There's no need for that, return value means integer item index. What
>>> are you fixing with that?
>> Hi Nikolay,
>>
>> The above change suppresses sign comparison warning in the line
>>
>> if (selitem != -1) ...
>>
>> and two more places below. selitem is declared as unsigned (DWORD), so
>> comparing with -1 produces warning. Since SendDlgItemMessageW anyway
>> return LRESULT which is signed integer, patch changes the variable type
>> to match function result and removes sign warnings.
> LRESULT is something that supposed to work anyway as I understand it,
> for this case I think it's
> better to cast function return value to INT and use INT as a local type.
Hi Nikolay,
I don't think that casting is a good practice, it could just hide warnings
instead of fixing them (ok, sometimes is necesary). However, in this case
LRESULT (Long result) is anyway an int, 32 or 64 bit, depending on the
platform.
As I can see it, using LRESULT or INT is the same. Or maybe I am missing
something else...
Marko
Re: appwiz.cpl: Removed sign comparison warning
Nikolay Sivov wrote:
> On 8/20/2010 20:04, Marko Nikolic wrote:
>> Changed variable type to match function return type.
>> ---
>> dlls/appwiz.cpl/appwiz.c |2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/dlls/appwiz.cpl/appwiz.c b/dlls/appwiz.cpl/appwiz.c
>> index ffd2b24..1b3370b 100644
>> --- a/dlls/appwiz.cpl/appwiz.c
>> +++ b/dlls/appwiz.cpl/appwiz.c
>> @@ -406,7 +406,7 @@ static void UpdateButtons(HWND hWnd)
>> {
>> APPINFO *iter;
>> LVITEMW lvItem;
>> -DWORD selitem = SendDlgItemMessageW(hWnd, IDL_PROGRAMS,
>> LVM_GETNEXTITEM, -1,
>> +LRESULT selitem = SendDlgItemMessageW(hWnd, IDL_PROGRAMS,
>> LVM_GETNEXTITEM, -1,
>> LVNI_FOCUSED | LVNI_SELECTED);
>> BOOL enable_modify = FALSE;
>>
> There's no need for that, return value means integer item index. What
> are you fixing with that?
Hi Nikolay,
The above change suppresses sign comparison warning in the line
if (selitem != -1) ...
and two more places below. selitem is declared as unsigned (DWORD), so
comparing with -1 produces warning. Since SendDlgItemMessageW anyway return
LRESULT which is signed integer, patch changes the variable type to match
function result and removes sign warnings.
BR,
Marko
Re: wineboot: fix Serbian Latin translation
Paul Vriens wrote: > On 07/14/2010 09:40 AM, Damjan Jovanovic wrote: >> Changelog: >> * wineboot: fix Serbian Latin translation >> >> Damjan Jovanovic >> > > Hi Damjan, > > Thanks for looking into this. I guess we need the pragma statement now > as you are introducing UTF-8, not? > > I did talk to Nenad about the fact that his translations appeared to be > all ASCII (and I did see some non-ASCII ones on wikipedia) but no > response so I'd figured it was correct. > Hi, No, it is not correct, Serbian Latin translation must include non-ASCII character. The article on wikipedia is correct. Regards, Marko
size_t usage in wine
Hi, I have found in wine documentation that it is recommended to avoid long data type, since it is different in Win64 and Unix64. This is reasonable, however, is there some similar policy regarding size_t/ssize_t usage? Those types are often defined as long type, so they could produce the same problem with 64 bit. On the other hand, many standard functions expect or return size_t values, so I guess it could be ok to use it in such cases. Is there any advice/position on this? Thanks, Marko
