Re: [ros-dev] [ros-diffs] [dchapyshev] 53519: - Fix multiple typos and bugs, found by PVS-Studio

2011-09-03 Thread Alex Ionescu
A lot of these are in 3rdparty/wine DLLs.

Were these bugs reported to the appropriate projects?

Best regards,
Alex Ionescu


On Thu, Sep 1, 2011 at 4:30 PM, dchapys...@svn.reactos.org wrote:

 Author: dchapyshev
 Date: Thu Sep  1 15:30:19 2011
 New Revision: 53519

 URL: http://svn.reactos.org/svn/reactos?rev=53519view=rev
 Log:
 - Fix multiple typos and bugs, found by PVS-Studio

 Modified:
trunk/reactos/base/applications/sndrec32/sndrec32.cpp
trunk/reactos/base/system/smss/client.c
trunk/reactos/dll/cpl/desk/screensaver.c
trunk/reactos/dll/win32/browseui/bandsite.cpp
trunk/reactos/dll/win32/glu32/libnurbs/internals/mapdesc.cc
trunk/reactos/dll/win32/oleaut32/typelib2.c
trunk/reactos/dll/win32/rsaenh/sha2.c
trunk/reactos/dll/win32/shell32/pidl.c
trunk/reactos/drivers/storage/ide/uniata/id_dma.cpp
trunk/reactos/ntoskrnl/config/cmcontrl.c
trunk/reactos/subsystems/win32/win32k/eng/gradient.c
trunk/reactos/subsystems/win32/win32k/objects/bitblt.c

 Modified: trunk/reactos/base/applications/sndrec32/sndrec32.cpp
 URL:
 http://svn.reactos.org/svn/reactos/trunk/reactos/base/applications/sndrec32/sndrec32.cpp?rev=53519r1=53518r2=53519view=diff

 ==
 --- trunk/reactos/base/applications/sndrec32/sndrec32.cpp [iso-8859-1]
 (original)
 +++ trunk/reactos/base/applications/sndrec32/sndrec32.cpp [iso-8859-1] Thu
 Sep  1 15:30:19 2011
 @@ -766,7 +766,7 @@
 isnew = TRUE;
 display_dur = TRUE;

 -ZeroMemory( file_path, MAX_PATH );
 +ZeroMemory( file_path, MAX_PATH * sizeof(TCHAR) );

 EnableWindow( slider, FALSE );


 Modified: trunk/reactos/base/system/smss/client.c
 URL:
 http://svn.reactos.org/svn/reactos/trunk/reactos/base/system/smss/client.c?rev=53519r1=53518r2=53519view=diff

 ==
 --- trunk/reactos/base/system/smss/client.c [iso-8859-1] (original)
 +++ trunk/reactos/base/system/smss/client.c [iso-8859-1] Thu Sep  1
 15:30:19 2011
 @@ -441,7 +441,7 @@
 */
RtlCopyMemory
 (SmpClientDirectory.CandidateClient-ProgramName,
   ProgramName,
 -  SM_SB_NAME_MAX_LENGTH);
 +  SM_SB_NAME_MAX_LENGTH *
 sizeof(WCHAR));
}
} else {
DPRINT1(SM: %s: CandidateClient %p pending!\n,
 __FUNCTION__,

 Modified: trunk/reactos/dll/cpl/desk/screensaver.c
 URL:
 http://svn.reactos.org/svn/reactos/trunk/reactos/dll/cpl/desk/screensaver.c?rev=53519r1=53518r2=53519view=diff

 ==
 --- trunk/reactos/dll/cpl/desk/screensaver.c [iso-8859-1] (original)
 +++ trunk/reactos/dll/cpl/desk/screensaver.c [iso-8859-1] Thu Sep  1
 15:30:19 2011
 @@ -450,7 +450,7 @@
 lpBackSlash = _tcsrchr(szSearchPath, _T('\\'));
 if (lpBackSlash != NULL)
 {
 -lpBackSlash = '\0';
 +*lpBackSlash = '\0';
 SearchScreenSavers(hwndScreenSavers, szSearchPath, pData);
 }
  }

 Modified: trunk/reactos/dll/win32/browseui/bandsite.cpp
 URL:
 http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/browseui/bandsite.cpp?rev=53519r1=53518r2=53519view=diff

 ==
 --- trunk/reactos/dll/win32/browseui/bandsite.cpp [iso-8859-1] (original)
 +++ trunk/reactos/dll/win32/browseui/bandsite.cpp [iso-8859-1] Thu Sep  1
 15:30:19 2011
 @@ -719,7 +719,7 @@
 if (fRebarWindow == NULL)
 return E_FAIL;

 -if (IsEqualIID(pguidCmdGroup, IID_IDeskBand))
 +if (IsEqualIID(*pguidCmdGroup, IID_IDeskBand))
 {
 switch (nCmdID)
 {

 Modified: trunk/reactos/dll/win32/glu32/libnurbs/internals/mapdesc.cc
 URL:
 http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/glu32/libnurbs/internals/mapdesc.cc?rev=53519r1=53518r2=53519view=diff

 ==
 --- trunk/reactos/dll/win32/glu32/libnurbs/internals/mapdesc.cc
 [iso-8859-1] (original)
 +++ trunk/reactos/dll/win32/glu32/libnurbs/internals/mapdesc.cc
 [iso-8859-1] Thu Sep  1 15:30:19 2011
 @@ -90,7 +90,7 @@
  }

  void
 -Mapdesc::identify( REAL dest[MAXCOORDS][MAXCOORDS] )
 +Mapdesc::identify( REAL (dest)[MAXCOORDS][MAXCOORDS] )
  {
 memset( dest, 0, sizeof( dest ) );
 for( int i=0; i != hcoords; i++ )

 Modified: trunk/reactos/dll/win32/oleaut32/typelib2.c
 URL:
 http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/oleaut32/typelib2.c?rev=53519r1=53518r2=53519view=diff

 ==
 --- trunk/reactos/dll/win32/oleaut32/typelib2.c [iso-8859-1] (original)
 +++ trunk/reactos/dll/win32/oleaut32/typelib2.c [iso-8859-1] Thu 

Re: [ros-dev] [ros-diffs] [cmihail] 53537: [shell32.dll] [FORMATTING] - Format the code to a more acceptable level. This is just for my sanity while sifting through it.

2011-09-03 Thread Claudiu Mihail
I know...I commited a reformat last night to remove the tabs. My bad.

Best regards,
Claudiu

On Sat, Sep 3, 2011 at 2:14 PM, Timo Kreuzer timo.kreu...@web.de wrote:

 Am 03.09.2011 02:08, schrieb cmih...@svn.reactos.org:

 Author: cmihail
 Date: Sat Sep  3 00:08:11 2011
 New Revision: 53537

 URL:http://svn.reactos.org/**svn/reactos?rev=53537view=revhttp://svn.reactos.org/svn/reactos?rev=53537view=rev
 Log:
 [shell32.dll]
 [FORMATTING]
 - Format the code to a more acceptable level. This is just for my sanity
 while sifting through it.


   /* skip the remaining spaces */
 -while (*cs==0x0009 || *cs==0x0020) {
 +while (*cs==0x0009 || *cs==0x0020)
 +   {
  cs++;
  }
  if (*cs==0)


 Please don't mix tabs and spaces or we'll end up looking like GNU style ;-)
 Since the file already uses spaces (as we use almost everywhere), please
 stick to that here.

 Regards,
 Timo


 __**_
 Ros-dev mailing list
 Ros-dev@reactos.org
 http://www.reactos.org/**mailman/listinfo/ros-devhttp://www.reactos.org/mailman/listinfo/ros-dev

___
Ros-dev mailing list
Ros-dev@reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev

Re: [ros-dev] [ros-diffs] [cmihail] 53543: [shell32.dll] - Fix bug related to correct error code returning in delete_files. The value of 1026 was revealed to be returned by windows 2003 server. Score

2011-09-03 Thread Jérôme Gardou

Hello Claudiu!

I notice some weird tab/spaces modifications here.

Regards.
Jérôme.

Le 03/09/2011 16:20, cmih...@svn.reactos.org a écrit :

Author: cmihail
Date: Sat Sep  3 14:20:03 2011
New Revision: 53543

URL: http://svn.reactos.org/svn/reactos?rev=53543view=rev
Log:
[shell32.dll]
- Fix bug related to correct error code returning in delete_files. The value of 
1026 was revealed to be returned by windows 2003 server. Score several passed 
winetests.
- Fix a bug in ShellLink::SetShowCmd and score one more passed winetest

Modified:
 branches/shell32_new-bringup/dll/win32/shell32/shelllink.cpp
 branches/shell32_new-bringup/dll/win32/shell32/shlfileop.cpp





___
Ros-dev mailing list
Ros-dev@reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev

Re: [ros-dev] [ros-diffs] [jgardou] 53547: [VGA_NEW] - fix some warnings

2011-09-03 Thread Alex Ionescu
These changes are wrong :(

Please use the correct compiler flag instead (-mrtd)

Best regards,
Alex Ionescu


On Sat, Sep 3, 2011 at 5:09 PM, jgar...@svn.reactos.org wrote:

 Author: jgardou
 Date: Sat Sep  3 16:09:34 2011
 New Revision: 53547

 URL: http://svn.reactos.org/svn/reactos?rev=53547view=rev
 Log:
 [VGA_NEW]
 - fix some warnings

 Modified:
trunk/reactos/drivers/video/miniport/vga_new/vga.c

 Modified: trunk/reactos/drivers/video/miniport/vga_new/vga.c
 URL:
 http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/video/miniport/vga_new/vga.c?rev=53547r1=53546r2=53547view=diff

 ==
 --- trunk/reactos/drivers/video/miniport/vga_new/vga.c [iso-8859-1]
 (original)
 +++ trunk/reactos/drivers/video/miniport/vga_new/vga.c [iso-8859-1] Sat Sep
  3 16:09:34 2011
 @@ -19,6 +19,7 @@
  //

  VP_STATUS
 +NTAPI
  VgaFindAdapter(
 PVOID HwDeviceExtension,
 PVOID HwContext,
 @@ -28,11 +29,13 @@
 );

  BOOLEAN
 +NTAPI
  VgaInitialize(
 PVOID HwDeviceExtension
 );

  BOOLEAN
 +NTAPI
  VgaStartIO(
 PVOID HwDeviceExtension,
 PVIDEO_REQUEST_PACKET RequestPacket
 @@ -102,6 +105,7 @@
 );

  VP_STATUS
 +NTAPI
  GetDeviceDataCallback(
PVOID HwDeviceExtension,
PVOID Context,
 @@ -130,7 +134,6 @@
  #pragma alloc_text(PAGE,VgaSetColorLookup)
  #endif

 -

  //---
  ULONG
  // eVb: 1.3 [GCC] - Add NTAPI for GCC support
 @@ -303,9 +306,10 @@
 return initializationStatus;

  } // end DriverEntry()
 -
 +

  //---
  VP_STATUS
 +NTAPI
  VgaFindAdapter(
 PVOID HwDeviceExtension,
 PVOID HwContext,
 @@ -383,7 +387,7 @@
 if ((ConfigInfo-AdapterInterfaceType == Internal) 
 (VideoPortGetDeviceData(HwDeviceExtension,
 VpControllerData,
 -GetDeviceDataCallback,
 +GetDeviceDataCallback,
 VgaAccessRange) != NO_ERROR))
 {
 return ERROR_INVALID_PARAMETER;
 @@ -485,9 +489,10 @@


  } // VgaFindAdapter()
 -
 +

  //---
  BOOLEAN
 +NTAPI
  VgaInitialize(
 PVOID HwDeviceExtension
 )
 @@ -527,9 +532,10 @@
 return TRUE;

  } // VgaInitialize()
 -
 +

  //---
  BOOLEAN
 +NTAPI
  VgaStartIO(
 PVOID HwDeviceExtension,
 PVIDEO_REQUEST_PACKET RequestPacket
 @@ -1440,8 +1446,9 @@
 return ERROR_INVALID_PARAMETER;

  } // end VgaSetColorLookup()
 -
 +
  VP_STATUS
 +NTAPI
  GetDeviceDataCallback(
 PVOID HwDeviceExtension,
 PVOID Context,



___
Ros-dev mailing list
Ros-dev@reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev

Re: [ros-dev] [ros-diffs] [jgardou] 53547: [VGA_NEW] - fix some warnings

2011-09-03 Thread Jérôme Gardou
Yuou're right... It was a bad mistake in my local copy which caused the 
warnings to appear.


Thanks for pointing this out.
BTW, I took a look at it, and our headers have the NTAPI calling 
convention, whereas WDK ones haven't.


Le 03/09/2011 18:21, Alex Ionescu a écrit :

These changes are wrong :(

Please use the correct compiler flag instead (-mrtd)

Best regards,
Alex Ionescu




___
Ros-dev mailing list
Ros-dev@reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev


Re: [ros-dev] [ros-diffs] [jgardou] 53547: [VGA_NEW] - fix some warnings

2011-09-03 Thread Timo Kreuzer

Am 03.09.2011 18:21, schrieb Alex Ionescu:

These changes are wrong :(

Please use the correct compiler flag instead (-mrtd)

Best regards,
Alex Ionescu


Can you explain what exactly is wrong with this change and why we use 
-mrtd here and in a few other places?
Is there anything better in using this (IMO hack) instead of using 
proper definitions, like we have elsewhere?


Oh, btw, -mrtd conflicts with some of out crt headers

Regards,
Timo


___
Ros-dev mailing list
Ros-dev@reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev


Re: [ros-dev] [ros-diffs] [dchapyshev] 53519: - Fix multiple typos and bugs, found by PVS-Studio

2011-09-03 Thread Thomas Faber
On 2011-09-03 20:07, Timo Kreuzer wrote:
 Am 03.09.2011 13:14, schrieb Alex Ionescu:
 --- trunk/reactos/dll/win32/browseui/bandsite.cpp [iso-8859-1] (original)
 +++ trunk/reactos/dll/win32/browseui/bandsite.cpp [iso-8859-1] Thu Sep  1 
 15:30:19 2011
 @@ -719,7 +719,7 @@
 if (fRebarWindow == NULL)
 return E_FAIL;

 -if (IsEqualIID(pguidCmdGroup, IID_IDeskBand))
 +if (IsEqualIID(*pguidCmdGroup, IID_IDeskBand))
 {
 switch (nCmdID)
 {
 This looks wrong, IsEqualIID accepts GUID*, not GUID

By chance, we've discussed this stuff for shell earlier today.
REFIID is a reference for C++.
I don't know who came up with that ingenious definition, but it should
make this correct.

-Tom

___
Ros-dev mailing list
Ros-dev@reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev


Re: [ros-dev] [ros-diffs] [jgardou] 53547: [VGA_NEW] - fix some warnings

2011-09-03 Thread Dmitry Gorbachev
BTW, -mrtd has recently (in April) changed its behavior in GCC trunk.
If reactos needs this flag, then maybe somebody should ask the author
of that change (Kai) to provide another flag (say, -mold-rtd) instead
of it.

___
Ros-dev mailing list
Ros-dev@reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev


Re: [ros-dev] [ros-diffs] [jgardou] 53547: [VGA_NEW] - fix some warnings

2011-09-03 Thread Jérôme Gardou

May I ask what the change is?

Le 03/09/2011 22:53, Dmitry Gorbachev a écrit :

BTW, -mrtd has recently (in April) changed its behavior in GCC trunk.
If reactos needs this flag, then maybe somebody should ask the author
of that change (Kai) to provide another flag (say, -mold-rtd) instead
of it.

___
Ros-dev mailing list
Ros-dev@reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev



___
Ros-dev mailing list
Ros-dev@reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev


Re: [ros-dev] [ros-diffs] [dchapyshev] 53519: - Fix multiple typos and bugs, found by PVS-Studio

2011-09-03 Thread Timo Kreuzer

Am 03.09.2011 21:50, schrieb Thomas Faber:


By chance, we've discussed this stuff for shell earlier today.
REFIID is a reference for C++.
I don't know who came up with that ingenious definition, but it should
make this correct.

-Tom



Ok, one more thing to write in my big black book of c++ ;-)






___
Ros-dev mailing list
Ros-dev@reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev


Re: [ros-dev] [ros-diffs] [jgardou] 53547: [VGA_NEW] - fix some warnings

2011-09-03 Thread Dmitry Gorbachev
 May I ask what the change is?

With -mrtd, it now does not add @n suffix to stdcall functions, and
also assumes that libgcc functions are stdcall (that they pop
arguments).

___
Ros-dev mailing list
Ros-dev@reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev


Re: [ros-dev] [ros-diffs] [jgardou] 53547: [VGA_NEW] - fix some warnings

2011-09-03 Thread Alex Ionescu
That's always been the case, and easily fixable with the switch I mentioned
which disables libgcc built-in functions (which shouldn't be used anyway).

For exported functions, I think MS always uses explicit convention in their
headers and sample source, so it shouldn't be a problem.

Best regards,
Alex Ionescu


On Sat, Sep 3, 2011 at 11:32 PM, Dmitry Gorbachev gorbac...@reactos.orgwrote:

  May I ask what the change is?

 With -mrtd, it now does not add @n suffix to stdcall functions, and
 also assumes that libgcc functions are stdcall (that they pop
 arguments).

 ___
 Ros-dev mailing list
 Ros-dev@reactos.org
 http://www.reactos.org/mailman/listinfo/ros-dev

___
Ros-dev mailing list
Ros-dev@reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev

Re: [ros-dev] [ros-diffs] [jgardou] 53547: [VGA_NEW] - fix some warnings

2011-09-03 Thread Dmitry Gorbachev
 That's always been the case,

At least in 4.4, 4.5, and 4.6, as I see, libgcc functions are assumed
to be cdecl even with -mrtd.

 and easily fixable with the switch I mentioned
 which disables libgcc built-in functions (which shouldn't be used anyway).

I don't know any GCC switch which disables calls to libgcc functions.
These are used, for example, to implement long long integer division
(happens in classpnp, and it's compiled with -mrtd).

-fno-builtin, it disables built-in functions such as cos or strlen.

 For exported functions, I think MS always uses explicit convention in their
 headers and sample source, so it shouldn't be a problem.

But now with -mrtd, the suffix is not added even to functions
explicitly declared as stdcall.

___
Ros-dev mailing list
Ros-dev@reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev


Re: [ros-dev] [ros-diffs] [jgardou] 53547: [VGA_NEW] - fix some warnings

2011-09-03 Thread Dmitry Gorbachev
 At least in 4.4, 4.5, and 4.6, as I see, libgcc functions are assumed
 to be cdecl even with -mrtd.

I was wrong about 4.7; it works well, too.

 But now with -mrtd, the suffix is not added even to functions
 explicitly declared as stdcall.

This observation is still valid.

___
Ros-dev mailing list
Ros-dev@reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev