Ros-dev August 2010

ros-dev@reactos.org

18 participants
18 discussions

[ros-diffs] [akhaldi] 48502: [PSDK] - Move INPUTCONTEXT and ImmLockIMC to the wine imm.h - Add wingdi.h inclusion.
by James Tabor 10 Aug '10

10 Aug '10

Hi! Should INPUTCONTEXT be moved or added to dimm.h? Wine headers should be used for wine specific stuff not DDK, SDK or WDK.... James Reference: http://msdn.microsoft.com/en-us/library/aa741229(VS.85).aspx

1 0

Re: [ros-dev] [ros-diffs] [sir_richard] 48475: [KERNEL32]: While working on the CMAKE branch, Amine and myself discovered a rather serious issue in kernel32 (and perhaps other libraries as well). Unlike rbuild, CMake does not allow you to export non-existant DLL functions (try it: add "poopyhead" in kernel32's exports under RBuild, and will it export "poopyhead", God knowing what that will actually link to). As an additional feature on top of the "allow non-existing functions to be exported" "feature", beca
by Timo Kreuzer 08 Aug '10

08 Aug '10

sir_richard(a)svn.reactos.org wrote: > Author: sir_richard > Date: Sat Aug 7 05:02:58 2010 > New Revision: 48475 > > URL: http://svn.reactos.org/svn/reactos?rev=48475&view=rev > Log: > [KERNEL32]: While working on the CMAKE branch, Amine and myself discovered a rather serious issue in kernel32 (and perhaps other libraries as well). I'm sure noone else could have discovered this issue before. cmake was the key! ;-) > Unlike rbuild, CMake does not allow you to export non-existant DLL functions (try it: add "poopyhead" in kernel32's exports under RBuild, and will it export "poopyhead", God knowing what that will actually link to). > That's an undocumented rbuild feature, called api-out-of-thin-air. To understand how it works, you must study quantum chaos theory. > there already exists a function for: *RtlSetLastNtStatusFromWin32Error*. > I see, with cmake, this feature works on ntdll as well ;-p Regards, Timo PS: build is broken.

2 1

07 Aug '10

I have a couple of questions/suggestions. Even if it was a mistake to commit it to trunk, I still express my opinion because I think it's important for the changes to stay in trunk. 1. Channel-based system is preferred to the plain NDEBUG/DPRINT one. Kernel32 should not really be an exception. If there is something wrong, it's better to put some more time into it and fix the actual problem in that debug library. Everyone who ever tried to debug any usermode app in ROS, would understand. 2. .pspec file was also invented as a consistent way to be compatible with different compilers. Winebuild generates stubs for entries marked as "stub", so it's quite controllable. 3. Everything else is great, nice to see that mess being finally cleaned up. WBR, Aleksey Bragin. On Aug 7, 2010, at 9:02 AM, sir_richard(a)svn.reactos.org wrote: > Author: sir_richard > Date: Sat Aug 7 05:02:58 2010 > New Revision: 48475 > > URL: http://svn.reactos.org/svn/reactos?rev=48475&view=rev > Log: > [KERNEL32]: While working on the CMAKE branch...

2 1

Re: [ros-dev] [ros-diffs] [jgardou] 48465: [WIN32K] - Rewrite NtGdiStretchDIBitsInternal : clearer, faster, stronger (+1 wine test)
by Timo Kreuzer 06 Aug '10

06 Aug '10

Hi, Please take care about proper protection of the user mode buffer. The current solution: probe and forget is not safe. Possibilities are: 1) SEH protected copying of the buffer, pass the copy of the buffer to lower level functions -> Easy to do, large overhead for large bitmaps. 2) SEH protected call to a lower level function, passing the user mode buffer. -> Not possible if the lower level function is either allocating any resources (unless also protected by SEH + finally) or can pass execution to 3rd party provided code, like drivers. 3) Be sure to have SEH at the lowest level (DIB) -> Not possible as the function might end up in a driver. 4) Use Mm to protect the buffer. Either with MmSecureVirtualMemory or double mapping using MmProbeAndLockPages + MmGetSystemAddressForMdlSafe. I think 4 is the way to go. While the overhead of remapping should be relatively small compared to a full copy, we are still wasting large ammounts of system address space. MmSecureVirtualMemory might at first sound like a good solution, but beware, it has some pitfalls. While it protects a memory range from being freed, it doesn't protect it from being paged out. That wouldn't be a problem, unless the memory is not backed by the page file, but let say a network resource, which becomes unavailable after a page was paged out. In this case we would get an in page error when trying to access the page, leading to a kernel crash. So unless we can be sure that the memory is backed by the page file, we need to additionally lock the pages into memory to be safe. Final thing to note is that MmSecureVirtualMemory is not implemented yet, but I hope with current work on the VAD code, we'll soon get a present (hint). Regards, Timo jgardou(a)svn.reactos.org wrote: > Author: jgardou > Date: Thu Aug 5 21:12:57 2010 > New Revision: 48465 > > URL: http://svn.reactos.org/svn/reactos?rev=48465&view=rev > Log: > [WIN32K] > - Rewrite NtGdiStretchDIBitsInternal : clearer, faster, stronger (+1 wine test) > > Modified: > branches/reactos-yarotows/subsystems/win32/win32k/objects/dibobj.c > > Modified: branches/reactos-yarotows/subsystems/win32/win32k/objects/dibobj.c > URL: http://svn.reactos.org/svn/reactos/branches/reactos-yarotows/subsystems/win… > ============================================================================== > --- branches/reactos-yarotows/subsystems/win32/win32k/objects/dibobj.c [iso-8859-1] (original) > +++ branches/reactos-yarotows/subsystems/win32/win32k/objects/dibobj.c [iso-8859-1] Thu Aug 5 21:12:57 2010 > @@ -290,7 +290,7 @@ > > if(!(psurfSrc && psurfDst)) > { > - DPRINT1("Error, could not lock surfaces.\n"); > + DPRINT1("Error, could not lock surfaces\n"); > goto cleanup; > } > > @@ -663,15 +663,15 @@ > if(pbmci) > { > pbmci->bmciHeader.bcWidth = psurf->SurfObj.sizlBitmap.cx; > - pbmci->bmciHeader.bcHeight = (psurf->SurfObj.fjBitmap & BMF_TOPDOWN) ? > - -psurf->SurfObj.sizlBitmap.cy : > + pbmci->bmciHeader.bcHeight = (psurf->SurfObj.fjBitmap & BMF_TOPDOWN) ? > + -psurf->SurfObj.sizlBitmap.cy : > psurf->SurfObj.sizlBitmap.cy; > pbmci->bmciHeader.bcPlanes = 1; > pbmci->bmciHeader.bcBitCount = BitsPerFormat(psurf->SurfObj.iBitmapFormat); > } > Info->bmiHeader.biWidth = psurf->SurfObj.sizlBitmap.cx; > - Info->bmiHeader.biHeight = (psurf->SurfObj.fjBitmap & BMF_TOPDOWN) ? > - -psurf->SurfObj.sizlBitmap.cy : > + Info->bmiHeader.biHeight = (psurf->SurfObj.fjBitmap & BMF_TOPDOWN) ? > + -psurf->SurfObj.sizlBitmap.cy : > psurf->SurfObj.sizlBitmap.cy;; > Info->bmiHeader.biPlanes = 1; > Info->bmiHeader.biBitCount = BitsPerFormat(psurf->SurfObj.iBitmapFormat); > @@ -711,15 +711,15 @@ > case 8: > Info->bmiHeader.biClrUsed = 0; > > - /* If the bitmap if a DIB section and has the same format than what > + /* If the bitmap if a DIB section and has the same format than what > * we're asked, go ahead! */ > - if((psurf->hSecure) && > + if((psurf->hSecure) && > (BitsPerFormat(psurf->SurfObj.iBitmapFormat) == bpp)) > { > if(Usage == DIB_RGB_COLORS) > { > unsigned int colors = min(psurf->ppal->NumColors, 1 << bpp); > - > + > if(pbmci) > { > for(i=0; i < colors; i++) > @@ -773,7 +773,7 @@ > rgbTriples[i].rgbtGreen = pDcPal->IndexedColors[i].peGreen; > rgbTriples[i].rgbtBlue = pDcPal->IndexedColors[i].peBlue; > } > - > + > rgbQuads[i].rgbRed = pDcPal->IndexedColors[i].peRed; > rgbQuads[i].rgbGreen = pDcPal->IndexedColors[i].peGreen; > rgbQuads[i].rgbBlue = pDcPal->IndexedColors[i].peBlue; > @@ -822,7 +822,7 @@ > { > for(g = 0; g <= 5; g++) > { > - for(b = 0; b <= 5; b++) > + for(b = 0; b <= 5; b++) > { > colorTriple->rgbtRed = (r * 0xff) / 5; > colorTriple->rgbtGreen = (g * 0xff) / 5; > @@ -935,7 +935,7 @@ > /* Restore them */ > Info->bmiHeader.biWidth = width; > Info->bmiHeader.biHeight = height; > - > + > if(!hBmpDest) > { > DPRINT1("Unable to create a DIB Section!\n"); > @@ -972,7 +972,7 @@ > _SEH2_TRY > { > RtlCopyMemory(Bits, pDIBits, DIB_GetDIBImageBytes (width, height, bpp)); > - } > + } > _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER) > { > Status = _SEH2_GetExceptionCode(); > @@ -1021,95 +1021,104 @@ > UINT cjMaxBits, > HANDLE hcmXform) > { > - HBITMAP hBitmap, hOldBitmap = NULL; > - HDC hdcMem = NULL; > + PDC pdc; > + INT ret = 0; > + LONG height; > + LONG width; > + WORD planes, bpp; > + DWORD compr, size; > + HBITMAP hBitmap; > + BOOL fastpath = FALSE; > NTSTATUS Status = STATUS_SUCCESS; > - PVOID pvDIBits; > - INT Ret = 0; > + > > if (!Bits || !BitsInfo) > - { > - SetLastWin32Error(ERROR_INVALID_PARAMETER); > return 0; > - } > - > - /* Create a DIB Section, data will be probed there */ > - hBitmap = NtGdiCreateDIBSection(hDC, > - NULL, > - 0, > - BitsInfo, > - Usage, > - 0, > - 0, > - 0, > - &pvDIBits); > - > - if(!hBitmap) > - { > - DPRINT1("Failed to create a DIB.\n"); > - return 0; > - } > - > - /* Set bits */ > - _SEH2_TRY > - { > - ProbeForRead(Bits, cjMaxBits, 1); > - RtlCopyMemory(pvDIBits, Bits, DIB_GetDIBImageBytes(BitsInfo->bmiHeader.biWidth, > - BitsInfo->bmiHeader.biHeight, > - BitsInfo->bmiHeader.biBitCount)); > - } > - _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER) > - { > - Status = _SEH2_GetExceptionCode(); > - } > - _SEH2_END > - > - if(!NT_SUCCESS(Status)) > - { > - DPRINT1("Error : Could not read DIB bits\n"); > - SetLastNtError(Status); > - goto cleanup; > - } > - > - hdcMem = NtGdiCreateCompatibleDC(0); > - if(!hdcMem) > - { > - DPRINT1("Failed to create a memory DC!"); > - goto cleanup; > - } > - > - hOldBitmap = NtGdiSelectBitmap(hdcMem, hBitmap); > - if(!hOldBitmap) > - { > - DPRINT1("Could not select the DIB into the memory DC\n"); > - goto cleanup; > - } > - > - /* Do we want to stretch ? */ > - if((SrcWidth == DestWidth) && (SrcHeight == DestHeight)) > - { > - Ret = NtGdiBitBlt(hDC, XDest, YDest, XDest + DestWidth, YDest + DestHeight, > - hdcMem, XSrc, YSrc, ROP, 0, 0); > - } > - else > - { > - Ret = NtGdiStretchBlt(hDC, XDest, YDest, XDest + DestWidth, YDest + DestHeight, > - hdcMem, XSrc, YSrc, XSrc + SrcWidth, YSrc + SrcHeight, > - ROP, 0); > - } > - > - if(Ret) > - Ret = SrcHeight ; > - > + > + if (!(pdc = DC_LockDc(hDC))) return 0; > + > + > + _SEH2_TRY > + { > + ProbeForRead(BitsInfo, cjMaxInfo, 1); > + ProbeForRead(Bits, cjMaxBits, 1); > + if (DIB_GetBitmapInfo( &BitsInfo->bmiHeader, &width, &height, &planes, &bpp, &compr, &size ) == -1) > + { > + DPRINT1("Invalid bitmap\n"); > + Status = STATUS_INVALID_PARAMETER; > + } > + } > + _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER) > + { > + Status = _SEH2_GetExceptionCode(); > + } > + _SEH2_END > + > + if(!NT_SUCCESS(Status)) > + { > + DPRINT1("Error, failed to read the DIB bits\n"); > + goto cleanup; > + } > + > + if (width < 0) > + { > + DPRINT1("Bitmap has a negative width\n"); > + return 0; > + } > + > + hBitmap = NtGdiGetDCObject(hDC, OBJ_BITMAP); > + > + if (XDest == 0 && YDest == 0 && XSrc == 0 && XSrc == 0 && > + DestWidth == SrcWidth && DestHeight == SrcHeight && > + compr == BI_RGB && > + ROP == SRCCOPY) > + { > + BITMAP bmp; > + if (IntGdiGetObject(hBitmap, sizeof(bmp), &bmp) == sizeof(bmp)) > + { > + if (bmp.bmBitsPixel == bpp && > + bmp.bmWidth == SrcWidth && > + bmp.bmHeight == SrcHeight && > + bmp.bmPlanes == planes) > + fastpath = TRUE; > + } > + } > + > + if (fastpath) > + { > + /* fast path */ > + DPRINT1("using fast path\n"); > + ret = IntSetDIBits( pdc, hBitmap, 0, height, Bits, BitsInfo, Usage); > + } > + else > + { > + /* slow path - need to use StretchBlt */ > + HBITMAP hOldBitmap; > + HPALETTE hpal = NULL; > + HDC hdcMem; > + PVOID pvBits; > + > + hdcMem = NtGdiCreateCompatibleDC( hDC ); > + hBitmap = DIB_CreateDIBSection(pdc, BitsInfo, Usage, &pvBits, NULL, 0, 0); > + RtlCopyMemory(pvBits, Bits, cjMaxBits); > + hOldBitmap = NtGdiSelectBitmap( hdcMem, hBitmap ); > + > + /* Origin for DIBitmap may be bottom left (positive biHeight) or top > + left (negative biHeight) */ > + ret = NtGdiStretchBlt( hDC, XDest, YDest, DestWidth, DestHeight, > + hdcMem, XSrc, abs(height) - SrcHeight - YSrc, > + SrcWidth, SrcHeight, ROP, 0 ); > + if(hpal) > + GdiSelectPalette(hdcMem, hpal, FALSE); > + if(ret) > + ret = SrcHeight; > + NtGdiSelectBitmap( hdcMem, hOldBitmap ); > + NtGdiDeleteObjectApp( hdcMem ); > + GreDeleteObject( hBitmap ); > + } > cleanup: > - if(hdcMem) > - { > - if(hOldBitmap) NtGdiSelectBitmap(hdcMem, hOldBitmap); > - NtGdiDeleteObjectApp(hdcMem); > - } > - GreDeleteObject(hBitmap); > - > - return Ret; > + DC_UnlockDc(pdc); > + return ret; > } > > > @@ -1462,7 +1471,7 @@ > hpal = (HPALETTE) 0xFFFFFFFF; > } > } > - else > + else > { > hpal = BuildDIBPalette(bmi); > } > @@ -1563,7 +1572,7 @@ > * Get the info from a bitmap header. > * Return 0 for COREHEADER, 1 for INFOHEADER, -1 for error. > */ > -int > +int > FASTCALL > DIB_GetBitmapInfo( const BITMAPINFOHEADER *header, LONG *width, > LONG *height, WORD *planes, WORD *bpp, DWORD *compr, DWORD *size ) > @@ -1575,7 +1584,7 @@ > *height = core->bcHeight; > *planes = core->bcPlanes; > *bpp = core->bcBitCount; > - *compr = 0; > + *compr = BI_RGB; > *size = 0; > return 0; > } > @@ -1692,10 +1701,10 @@ > ppalEntries[i].peBlue = 0; > ppalEntries[i].peFlags = 0; > } > - > + > lpIndex++; > } > - > + > hpal = PALETTE_AllocPalette(PAL_INDEXED, nNumColors, (ULONG*)ppalEntries, 0, 0, 0); > > ExFreePoolWithTag(ppalEntries, TAG_COLORMAP); > > > >

2 1

Re: [ros-dev] [ros-diffs] [tkreuzer] 48461: [NTOSKRNL] - Fixed FsRtlIsNameInExpression to make it properly handle * in expressions - Fix formatting - Patch by Pierre Schweitzer - Fixes everything See issue #5541 for more details.
by Colin Finck 05 Aug '10

05 Aug '10

tkreuzer(a)svn.reactos.org wrote: > Author: tkreuzer > Date: Thu Aug 5 12:23:23 2010 > New Revision: 48461 > > URL: http://svn.reactos.org/svn/reactos?rev=48461&view=rev > Log: > [NTOSKRNL] > - Fixed FsRtlIsNameInExpression to make it properly handle * in expressions > - Fix formatting > - Patch by Pierre Schweitzer > - Fixes everything Could you add a test for this to one of our (Winetest-compatible) kernel-mode testing frameworks, so that nobody breaks this again? I know that we haven't decided on a radical test-based development approach yet, but it would certainly be helpful to avoid long trunk locks in the future. Cheers, Colin

1 0

05 Aug '10

1 0

Trunk locked
by Aleksey Bragin 02 Aug '10

02 Aug '10

Hi, trunk is locked due to reaching critical level of regressions again. Currently the following people have write access: tkreuzer, sir_richard, fireball. If you would like to be on bugfix-committers team, please drop me a line. WBR, Aleksey Bragin.

3 2

[ros-diffs] [jgardou] 48359: [WIN32K] - Rewrite NtGdiGetDIBitsInternal, with WINE as a reference. - Get back DIB Section creation to classic BITMAPINFO. This si the beginnig of the end for all this BITMAPV5INFO stuff. It is horrible, BITMAPCOREINFO->
by James Tabor 01 Aug '10

01 Aug '10

Hi! BitmapCoreInfo !Should Be! handled in Gdi32 and converted there! The latest BitmapInfo "must be" handled in win32k. So not get it confused. James

3 4

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

2004

Ros-dev August 2010