Ros-dev February 2015

ros-dev@reactos.org

15 participants
32 discussions

win32k C++ suggestion
by Timo Kreuzer 22 Feb '15

22 Feb '15

Hi I'd like to propose introducing C++ to win32k. Don't worry, this is not a suggestion to rewrite everything from scratch in C++, but to gradually introduce C++. The reason is not "That's what Windows does", but the fact that especially GDI would heavily benefit in terms of code simplicity, clarily and quality from using C++. A lot of the interfaces are already in a C++ object style way, but still code can bypass interfaces and directly modify structure members, even if it is not supposed to be done that way. Unless there are strong objections, I'd post a design / style / roadmap suggestion soon. If people feel strongly about it, we can defer this to after a 0.4.0 release. Regards, Timo

4 3

Re: [ros-dev] [ros-diffs] [jgardou] 66334: [NTOSKRNL/MM] - MiIsEntireRangeCommitted: Ensure the PTE we are checking is really faulted in. - Prefer MiPteToPde and MiPdeToPte (which should really be called MiFirstPteInPde) in...
by Alex Ionescu 21 Feb '15

21 Feb '15

it might fix an assert but the patch is incorrect. will this also take 6 months to revert? Best regards, Alex Ionescu On Tue, Feb 17, 2015 at 6:19 AM, <jgardou(a)svn.reactos.org> wrote: > Author: jgardou > Date: Tue Feb 17 14:19:05 2015 > New Revision: 66334 > > URL: http://svn.reactos.org/svn/reactos?rev=66334&view=rev > Log: > [NTOSKRNL/MM] > - MiIsEntireRangeCommitted: Ensure the PTE we are checking is really > faulted in. > - Prefer MiPteToPde and MiPdeToPte (which should really be called > MiFirstPteInPde) instead of MiAddressToPte and MiPteToAddress > Fixes weird failed ASSERT in page fault handler when using DPH. > > Modified: > trunk/reactos/ntoskrnl/mm/ARM3/virtual.c > > Modified: trunk/reactos/ntoskrnl/mm/ARM3/virtual.c > URL: > http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/ARM3/virtual.c… > > ============================================================================== > --- trunk/reactos/ntoskrnl/mm/ARM3/virtual.c [iso-8859-1] (original) > +++ trunk/reactos/ntoskrnl/mm/ARM3/virtual.c [iso-8859-1] Tue Feb 17 > 14:19:05 2015 > @@ -1994,14 +1994,13 @@ > if (OnBoundary) > { > /* Is this PDE demand zero? */ > - PointerPde = MiAddressToPte(PointerPte); > + PointerPde = MiPteToPde(PointerPte); > if (PointerPde->u.Long != 0) > { > /* It isn't -- is it valid? */ > if (PointerPde->u.Hard.Valid == 0) > { > /* Nope, fault it in */ > - PointerPte = MiPteToAddress(PointerPde); > MiMakeSystemAddressValid(PointerPte, Process); > } > } > @@ -2009,13 +2008,13 @@ > { > /* The PTE was already valid, so move to the next one */ > PointerPde++; > - PointerPte = MiPteToAddress(PointerPde); > + PointerPte = MiPdeToPte(PointerPde); > > /* Is the entire VAD committed? If not, fail */ > if (!Vad->u.VadFlags.MemCommit) return FALSE; > > - /* Everything is committed so far past the range, return > true */ > - if (PointerPte > LastPte) return TRUE; > + /* New loop iteration with our new, on-boundary PTE. */ > + continue; > } > } > > > >

4 4

Re: [ros-dev] [ros-diffs] [tfaber] 66365: [KERNEL32] - Make BaseSetLastNTError return the converted Win32 error code. This will determine the upper 24 bits of EAX in functions that return BOOLEAN FALSE right after calling B...
by Timo Kreuzer 21 Feb '15

21 Feb '15

For me this looks like a hack. It might "do what Windows does", but the result is more based on luck / random compiler behaviour rather than deterministic behavior. We should think about returning a full ULONG in all functions that rely on this (like Wow64EnableWow64FsRedirection), containing the correct value, rather than relying on random stuff. Am 20.02.2015 um 08:03 schrieb tfaber(a)svn.reactos.org: > Author: tfaber > Date: Fri Feb 20 07:03:00 2015 > New Revision: 66365 > > URL: http://svn.reactos.org/svn/reactos?rev=66365&view=rev > Log: > [KERNEL32] > - Make BaseSetLastNTError return the converted Win32 error code. This will determine the upper 24 bits of EAX in functions that return BOOLEAN FALSE right after calling BaseSetLastNTError, e.g. Wow64EnableWow64FsRedirection. Fixes installers using WiX Toolset (e.g. VS2012 redist) on MSVC builds. > See http://wixtoolset.org/issues/4681/ for the WiX bug that causes this. > CORE-8010 > > Modified: > trunk/reactos/dll/win32/kernel32/client/except.c > trunk/reactos/dll/win32/kernel32/include/kernel32.h > > Modified: trunk/reactos/dll/win32/kernel32/client/except.c > URL: http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/kernel32/client/… > ============================================================================== > --- trunk/reactos/dll/win32/kernel32/client/except.c [iso-8859-1] (original) > +++ trunk/reactos/dll/win32/kernel32/client/except.c [iso-8859-1] Fri Feb 20 07:03:00 2015 > @@ -682,12 +682,16 @@ > /* > * @implemented > */ > -VOID > +DWORD > WINAPI > BaseSetLastNTError(IN NTSTATUS Status) > { > + DWORD dwErrCode; > + > /* Convert from NT to Win32, then set */ > - SetLastError(RtlNtStatusToDosError(Status)); > + dwErrCode = RtlNtStatusToDosError(Status); > + SetLastError(dwErrCode); > + return dwErrCode; > } > > /* > > Modified: trunk/reactos/dll/win32/kernel32/include/kernel32.h > URL: http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/kernel32/include… > ============================================================================== > --- trunk/reactos/dll/win32/kernel32/include/kernel32.h [iso-8859-1] (original) > +++ trunk/reactos/dll/win32/kernel32/include/kernel32.h [iso-8859-1] Fri Feb 20 07:03:00 2015 > @@ -353,7 +353,7 @@ > WINAPI > InitCommandLines(VOID); > > -VOID > +DWORD > WINAPI > BaseSetLastNTError(IN NTSTATUS Status); > > > >

2 1

Re: [ros-dev] [ros-diffs] [gadamopoulos] 66383: [SHELL32] - Implement progress dialogs for SHFileOperation - Patch by Hwu Davies CORE-4476
by Pierre Schweitzer 21 Feb '15

21 Feb '15

In such situations, please update all the langages files and not only English. This makes work easier for translators to track changes. On 21/02/2015 13:52, gadamopoulos(a)svn.reactos.org wrote: > Author: gadamopoulos > Date: Sat Feb 21 12:52:58 2015 > New Revision: 66383 > > URL: http://svn.reactos.org/svn/reactos?rev=66383&view=rev > Log: > [SHELL32] > - Implement progress dialogs for SHFileOperation > - Patch by Hwu Davies > CORE-4476 > > Modified: > trunk/reactos/dll/win32/shell32/lang/en-US.rc > > Modified: trunk/reactos/dll/win32/shell32/lang/en-US.rc > URL: http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/shell32/lang/en-… > ============================================================================== > --- trunk/reactos/dll/win32/shell32/lang/en-US.rc [iso-8859-1] (original) > +++ trunk/reactos/dll/win32/shell32/lang/en-US.rc [iso-8859-1] Sat Feb 21 12:52:58 2015 > @@ -693,6 +693,13 @@ > IDS_OVERWRITEFILE_CAPTION "Confirm file overwrite" > IDS_OVERWRITEFOLDER_TEXT "This folder already contains a folder named '%1'.\n\nIf the files in the destination folder have the same names as files in the\nselected folder they will be replaced. Do you still want to move or copy\nthe folder?" > > + IDS_FILEOOP_COPYING "Copying" > + IDS_FILEOOP_MOVING "Moving" > + IDS_FILEOOP_DELETING "Deleting" > + IDS_FILEOOP_FROM_TO "From %1 to %2" > + IDS_FILEOOP_FROM "From %1" > + IDS_FILEOOP_PREFLIGHT "Preflight" > + > /* message box strings */ > IDS_RESTART_TITLE "Restart" > IDS_RESTART_PROMPT "Do you want to restart the system?" > -- Pierre Schweitzer <pierre at reactos.org> System & Network Administrator Senior Kernel Developer ReactOS Deutschland e.V.

1 0

Where is the Patchbot??
by Hermès BÉLUSCA - MAÏTO 21 Feb '15

21 Feb '15

Hi guys ! Where is the Patchbot?? Dont you think its still needed? Thanks, Hermès.

4 4

Re: [ros-dev] [jira] (ROSTESTS-153) gdi32_winetest:bitmap test_mono_bitmap skipped because it takes too long and spams the debug log with too many failures
by Thomas Faber 18 Feb '15

18 Feb '15

On 2015-02-16 10:27, zefklop (JIRA) wrote: > zefklop commented on ROSTESTS-153: > ---------------------------------- > > This and the advapi32:service disabled test call for (optional) per-test timeout parameter in rostests or testman.We can't afford skipping tests, that's an open door to all kind of regressions. I agree that skipping tests is a really bad idea. It should be used as a last resort only. And we need a proper process for it** -- I've been spending way too much time hunting down skipped tests recently, and way too many of them had their underlying bugs (and sometimes even Jira issue) fixed without re-enabling the test. However I don't see a practical way to implement your solution. For one, the timeout seems like it would have to cause a machine reboot, which significantly increases test time. I guess rosautotest could keep track of the time and kill the child process, but I don't know how reliable that would be. Secondly and more importantly, killing the test on timeout seems worse than skipping the offending part, because then any tests running _after_ the code causing the timeout would never be executed. That means instead of skipping a test that's known to be problematic, we skip completely unrelated and innocent tests. I'm not sure if there's a way to win this. Let me know if you have any ideas. As it stands I think skipping the offenders is an okay solution -- we just need to stay very aware of them and make sure to re-test more regularly than we do now. Thanks! -Thomas ** For the record, here's an example process that I try to keep to: - Create a ROSTESTS bug to indicate that a test was skipped - Link the ROSTESTS bug to ROSTESTS-125, the Skipped Tests Epic - Do not #if out the test, instead check for winetest_interactive, and skip the test only if it's not set - Make sure to call skip() with a message that references the ROSTESTS bug - Optionally create a CORE issue for the underlying problem if enough information is available; make sure it blocks the ROSTESTS issue - Do not under any circumstances close the ROSTESTS bug before the test code is actually restored and no longer skips the test in question This allows the issue to be actually found, by checking the Epic, or grepping for "ROSTESTS-" or "winetest_interactive". Skipped tests should be re-tested regularly, at the very least every time the test is Wine-synced. >> gdi32_winetest:bitmap test_mono_bitmap skipped because it takes too long and spams the debug log with too many failures >> ----------------------------------------------------------------------------------------------------------------------- >> >> Key: ROSTESTS-153 >> URL: https://jira.reactos.org/browse/ROSTESTS-153 >> Project: ReactOS Test Suite >> Issue Type: Bug >> Components: Wine Tests >> Reporter: Thomas Faber >> Assignee: Bug Zilla >> Labels: HACK >> >> If you close this task, that means the winetest_interactive check and skip() in test_mono_bitmap must be gone!

2 2

Re: [ros-dev] [ros-diffs] [jgardou] 66342: [WIN32SS/NTGDI] - Reject bits data early in SetBitmapBits when it's clear that they can't fit in the destination bitmap. Fixes ugly Office 2007 rulers showing garbage.
by Timo Kreuzer 18 Feb '15

18 Feb '15

Is this a bug in Office 2007 or are we doing something wrong here? Am 17.02.2015 um 21:41 schrieb jgardou(a)svn.reactos.org: > Author: jgardou > Date: Tue Feb 17 20:41:51 2015 > New Revision: 66342 > > URL: http://svn.reactos.org/svn/reactos?rev=66342&view=rev > Log: > [WIN32SS/NTGDI] > - Reject bits data early in SetBitmapBits when it's clear that they can't fit in the destination bitmap. > Fixes ugly Office 2007 rulers showing garbage. > > Modified: > trunk/reactos/win32ss/gdi/ntgdi/bitmaps.c > > Modified: trunk/reactos/win32ss/gdi/ntgdi/bitmaps.c > URL: http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/gdi/ntgdi/bitmaps.… > ============================================================================== > --- trunk/reactos/win32ss/gdi/ntgdi/bitmaps.c [iso-8859-1] (original) > +++ trunk/reactos/win32ss/gdi/ntgdi/bitmaps.c [iso-8859-1] Tue Feb 17 20:41:51 2015 > @@ -11,7 +11,8 @@ > #define NDEBUG > #include <debug.h> > > -void > +static > +int > NTAPI > UnsafeSetBitmapBits( > PSURFACE psurf, > @@ -32,6 +33,9 @@ > lDeltaDst = psurf->SurfObj.lDelta; > lDeltaSrc = WIDTH_BYTES_ALIGN16(nWidth, cBitsPixel); > > + if (cjBits && (cjBits < (lDeltaSrc * nHeight))) > + return 0; > + > while (nHeight--) > { > /* Copy one line */ > @@ -40,6 +44,7 @@ > pjDst += lDeltaDst; > } > > + return 1; > } > > HBITMAP > @@ -538,9 +543,8 @@ > > _SEH2_TRY > { > - ProbeForRead(pUnsafeBits, Bytes, 1); > - UnsafeSetBitmapBits(psurf, Bytes, pUnsafeBits); > - ret = 1; > + ProbeForRead(pUnsafeBits, Bytes, sizeof(WORD)); > + ret = UnsafeSetBitmapBits(psurf, Bytes, pUnsafeBits); > } > _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER) > { > > >

2 1

Re: [ros-dev] [ros-diffs] [jgardou] 66323: [KERNEL32] Revert r64525 - Always allocate a guard page at the bottom of the stack.
by Alex Ionescu 17 Feb '15

17 Feb '15

Finally... Best regards, Alex Ionescu On Mon, Feb 16, 2015 at 1:12 PM, <jgardou(a)svn.reactos.org> wrote: > Author: jgardou > Date: Mon Feb 16 21:12:51 2015 > New Revision: 66323 > > URL: http://svn.reactos.org/svn/reactos?rev=66323&view=rev > Log: > [KERNEL32] > Revert r64525 - Always allocate a guard page at the bottom of the stack. > > Modified: > trunk/reactos/dll/win32/kernel32/client/utils.c > > Modified: trunk/reactos/dll/win32/kernel32/client/utils.c > URL: > http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/kernel32/client/… > > ============================================================================== > --- trunk/reactos/dll/win32/kernel32/client/utils.c [iso-8859-1] > (original) > +++ trunk/reactos/dll/win32/kernel32/client/utils.c [iso-8859-1] Mon > Feb 16 21:12:51 2015 > @@ -358,6 +358,7 @@ > NTSTATUS Status; > PIMAGE_NT_HEADERS Headers; > ULONG_PTR Stack; > + BOOLEAN UseGuard; > ULONG PageSize, Dummy, AllocationGranularity; > SIZE_T StackReserveHeader, StackCommitHeader, GuardPageSize, > GuaranteedStackCommit; > DPRINT("BaseCreateStack (hProcess: %p, Max: %lx, Current: %lx)\n", > @@ -425,6 +426,18 @@ > /* Update the Stack Position */ > Stack += StackReserve - StackCommit; > > + /* Check if we will need a guard page */ > + if (StackReserve > StackCommit) > + { > + Stack -= PageSize; > + StackCommit += PageSize; > + UseGuard = TRUE; > + } > + else > + { > + UseGuard = FALSE; > + } > + > /* Allocate memory for the stack */ > Status = NtAllocateVirtualMemory(hProcess, > (PVOID*)&Stack, > @@ -444,21 +457,25 @@ > InitialTeb->StackLimit = (PVOID)Stack; > > /* Create a guard page */ > - GuardPageSize = PageSize; > - Status = NtProtectVirtualMemory(hProcess, > - (PVOID*)&Stack, > - &GuardPageSize, > - PAGE_GUARD | PAGE_READWRITE, > - &Dummy); > - if (!NT_SUCCESS(Status)) > - { > - DPRINT1("Failure to set guard page\n"); > - return Status; > - } > - > - /* Update the Stack Limit keeping in mind the Guard Page */ > - InitialTeb->StackLimit = (PVOID)((ULONG_PTR)InitialTeb->StackLimit + > - GuardPageSize); > + if (UseGuard) > + { > + /* Set the guard page */ > + GuardPageSize = PAGE_SIZE; > + Status = NtProtectVirtualMemory(hProcess, > + (PVOID*)&Stack, > + &GuardPageSize, > + PAGE_GUARD | PAGE_READWRITE, > + &Dummy); > + if (!NT_SUCCESS(Status)) > + { > + DPRINT1("Failure to set guard page\n"); > + return Status; > + } > + > + /* Update the Stack Limit keeping in mind the Guard Page */ > + InitialTeb->StackLimit = > (PVOID)((ULONG_PTR)InitialTeb->StackLimit + > + GuardPageSize); > + } > > /* We are done! */ > return STATUS_SUCCESS; > > >

1 0

Re: [ros-dev] [ros-diffs] [jimtabor] 66244: [NtUser] - Use a real timer for caret. This should cleanup message testing from those random system timer messages. See CORE-7447.
by Hermès BÉLUSCA - MAÏTO 13 Feb '15

13 Feb '15

Hi James! It would be nice if you could also do some cleanup in that caret code, because it seems (starting r66244): - there is somewhat duplicated caret painting in win32k and in user32, - the caret is sometimes drawn one or two pixels too high. And of course, everything should be done windows-compatible (not wine-compatible)!! Cheers, Hermès. ------------------------------------------ Author: jimtabor Date: Fri Feb 13 13:39:57 2015 New Revision: 66244 URL: http://svn.reactos.org/svn/reactos?rev=66244&view=rev Log: [NtUser] - Use a real timer for caret. This should cleanup message testing from those random system timer messages. See CORE-7447. Modified: trunk/reactos/win32ss/user/ntuser/caret.c Modified: trunk/reactos/win32ss/user/ntuser/caret.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/user/ntuser/caret.c ?rev=66244&r1=66243&r2=66244&view=diff ============================================================================ == --- trunk/reactos/win32ss/user/ntuser/caret.c [iso-8859-1] (original) +++ trunk/reactos/win32ss/user/ntuser/caret.c [iso-8859-1] Fri Feb 13 13:39:57 2015 @@ -16,6 +16,111 @@ /* FUNCTIONS *****************************************************************/ +VOID FASTCALL +co_IntDrawCaret(PWND pWnd, PTHRDCARETINFO CaretInfo) +{

2 1

Re: [ros-dev] [ros-diffs] [tkreuzer] 66250: [CMAKE] Add support for "module groups". These are meta targets that automatically include all targets using set_module_type() that are included between start_module_group(name) a...
by Jerome Gardou 13 Feb '15

13 Feb '15

Hey Timo, nice stuff, but I think you committed a bit too fast ;-) See inline: Le 13/02/2015 21:19, tkreuzer(a)svn.reactos.org a écrit : > Author: tkreuzer > Date: Fri Feb 13 20:19:51 2015 > New Revision: 66250 > > URL: http://svn.reactos.org/svn/reactos?rev=66250&view=rev > Log: > [CMAKE] > Add support for "module groups". These are meta targets that automatically include all targets using set_module_type() that are included between start_module_group(name) and end_module_group(). > > Modified: > trunk/reactos/cmake/CMakeMacros.cmake > > Modified: trunk/reactos/cmake/CMakeMacros.cmake > URL: http://svn.reactos.org/svn/reactos/trunk/reactos/cmake/CMakeMacros.cmake?re… > ============================================================================== > --- trunk/reactos/cmake/CMakeMacros.cmake [iso-8859-1] (original) > +++ trunk/reactos/cmake/CMakeMacros.cmake [iso-8859-1] Fri Feb 13 20:19:51 2015 > @@ -550,9 +550,14 @@ > message(STATUS "set_module_type : unparsed arguments ${__module_UNPARSED_ARGUMENTS}, module : ${MODULE}") > endif() > > + # Add the module to the module group list, if it is defined > + if(DEFINED CURRENT_MODULE_GROUP) > + set_property(GLOBAL APPEND PROPERTY ${CURRENT_MODULE_GROUP}_MODULE_LIST "${MODULE}") > + endif() > + > # Set subsystem. Also take this as an occasion > # to error out if someone gave a non existing type > - if((${TYPE} STREQUAL nativecui) OR (${TYPE} STREQUAL nativedll) > + if((${TYPE} STREQUAL nativecui) OR (${TYPE} STREQUAL nativedll) > OR (${TYPE} STREQUAL kernelmodedriver) OR (${TYPE} STREQUAL wdmdriver) OR (${TYPE} STREQUAL kerneldll)) > set(__subsystem native) > elseif(${TYPE} STREQUAL win32cui) > @@ -662,6 +667,21 @@ > > # do compiler specific stuff > set_module_type_toolchain(${MODULE} ${TYPE}) > +endfunction() > + > +function(start_module_group __name) > + if(DEFINED CURRENT_MODULE_GROUP) > + message(FATAL_ERROR "CURRENT_MODULE_GROUP is already set ('${CURRENT_MODULE_GROUP}')") > + endif() > + set(CURRENT_MODULE_GROUP rostests PARENT_SCOPE) > +endfunction() > + should be set(CURRENT_MODULE_GROUP ${__name} PARENT_SCOPE) > +function(end_module_group) > + get_property(__modulelist GLOBAL PROPERTY ${CURRENT_MODULE_GROUP}_MODULE_LIST) > + add_custom_target(${CURRENT_MODULE_GROUP}) > + foreach(__module ${__modulelist}) > + add_dependencies(${CURRENT_MODULE_GROUP} ${__module}) > + endforeach() > endfunction() You should unset CURRENT_MODULE_GROUP somewhere aroud here. > > function(preprocess_file __in __out) > >

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

2004

Ros-dev February 2015