Ros-dev September 2012

ros-dev@reactos.org

13 participants
23 discussions

by Timo Kreuzer

Hi, Does anyone know how to search for issues older than 1 year or something like that? According to https://jira.atlassian.com/browse/JRA-1983 it should be possible, but Icouldn't figure out how to. Timo

12 years, 10 months

Re: [ros-dev] [ros-diffs] [ion] 57284: [NTOSKRNL]: Use the token lock acquire/release macros that were already written instead of manually doing it. Also fix the macros since they didn't work in GCC. No functional change, j...

by Timo Kreuzer

What was the problem with the old macros? The new ones are error-prone. if (NeedLock) SepAcquireTokenLockExclusive(Token); // <= fail! WBR, Timo Am 12.09.2012 18:29, schrieb ion(a)svn.reactos.org: > Author: ion > Date: Wed Sep 12 16:29:28 2012 > New Revision: 57284 > > URL: http://svn.reactos.org/svn/reactos?rev=57284&view=rev > Log: > [NTOSKRNL]: Use the token lock acquire/release macros that were already written instead of manually doing it. Also fix the macros since they didn't work in GCC. > No functional change, just code cleanup. > > Modified: > trunk/reactos/ntoskrnl/include/internal/se.h > trunk/reactos/ntoskrnl/se/access.c > trunk/reactos/ntoskrnl/se/semgr.c > > Modified: trunk/reactos/ntoskrnl/include/internal/se.h > URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/include/internal/… > ============================================================================== > --- trunk/reactos/ntoskrnl/include/internal/se.h [iso-8859-1] (original) > +++ trunk/reactos/ntoskrnl/include/internal/se.h [iso-8859-1] Wed Sep 12 16:29:28 2012 > @@ -1,4 +1,28 @@ > #pragma once > + > +typedef struct _KNOWN_ACE > +{ > + ACE_HEADER Header; > + ACCESS_MASK Mask; > + ULONG SidStart; > +} KNOWN_ACE, *PKNOWN_ACE; > + > +typedef struct _KNOWN_OBJECT_ACE > +{ > + ACE_HEADER Header; > + ACCESS_MASK Mask; > + ULONG Flags; > + ULONG SidStart; > +} KNOWN_OBJECT_ACE, *PKNOWN_OBJECT_ACE; > + > +typedef struct _KNOWN_COMPOUND_ACE > +{ > + ACE_HEADER Header; > + ACCESS_MASK Mask; > + USHORT CompoundAceType; > + USHORT Reserved; > + ULONG SidStart; > +} KNOWN_COMPOUND_ACE, *PKNOWN_COMPOUND_ACE; > > PSID > FORCEINLINE > @@ -75,6 +99,8 @@ > return Descriptor->Sacl; > } > } > + > +#ifndef RTL_H > > /* SID Authorities */ > extern SID_IDENTIFIER_AUTHORITY SeNullSidAuthority; > @@ -156,6 +182,19 @@ > extern PSECURITY_DESCRIPTOR SeSystemDefaultSd; > extern PSECURITY_DESCRIPTOR SeUnrestrictedSd; > > + > +#define SepAcquireTokenLockExclusive(Token) \ > + KeEnterCriticalRegion(); \ > + ExAcquireResourceExclusive(((PTOKEN)Token)->TokenLock, TRUE); \ > + > +#define SepAcquireTokenLockShared(Token) \ > + KeEnterCriticalRegion(); \ > + ExAcquireResourceShared(((PTOKEN)Token)->TokenLock, TRUE); \ > + > +#define SepReleaseTokenLock(Token) \ > + ExReleaseResource(((PTOKEN)Token)->TokenLock); \ > + KeLeaveCriticalRegion(); \ > + > // > // Token Functions > // > @@ -434,24 +473,6 @@ > OUT PACCESS_TOKEN* NewToken > ); > > -#define SepAcquireTokenLockExclusive(Token) \ > - do { \ > - KeEnterCriticalRegion(); \ > - ExAcquireResourceExclusive(((PTOKEN)Token)->TokenLock, TRUE); \ > - while(0) > - > -#define SepAcquireTokenLockShared(Token) \ > - do { \ > - KeEnterCriticalRegion(); \ > - ExAcquireResourceShared(((PTOKEN)Token)->TokenLock, TRUE); \ > - while(0) > - > -#define SepReleaseTokenLock(Token) \ > - do { \ > - ExReleaseResource(((PTOKEN)Token)->TokenLock); \ > - KeLeaveCriticalRegion(); \ > - while(0) > - > VOID NTAPI > SeQuerySecurityAccessMask(IN SECURITY_INFORMATION SecurityInformation, > OUT PACCESS_MASK DesiredAccess); > @@ -460,4 +481,6 @@ > SeSetSecurityAccessMask(IN SECURITY_INFORMATION SecurityInformation, > OUT PACCESS_MASK DesiredAccess); > > +#endif > + > /* EOF */ > > Modified: trunk/reactos/ntoskrnl/se/access.c > URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/se/access.c?rev=5… > ============================================================================== > --- trunk/reactos/ntoskrnl/se/access.c [iso-8859-1] (original) > +++ trunk/reactos/ntoskrnl/se/access.c [iso-8859-1] Wed Sep 12 16:29:28 2012 > @@ -130,11 +130,7 @@ > ASSERT(Sid != NULL); > > /* Lock the token if needed */ > - if (!TokenLocked) > - { > - KeEnterCriticalRegion(); > - ExAcquireResourceSharedLite(Token->TokenLock, TRUE); > - } > + if (!TokenLocked) SepAcquireTokenLockShared(Token); > > /* Check if the owner SID is found, handling restricted case as well */ > Result = SepSidInToken(Token, Sid); > @@ -144,11 +140,7 @@ > } > > /* Release the lock if we had acquired it */ > - if (!TokenLocked) > - { > - ExReleaseResourceLite(Token->TokenLock); > - KeLeaveCriticalRegion(); > - } > + if (!TokenLocked) SepReleaseTokenLock(Token); > > /* Return the result */ > return Result; > @@ -168,15 +160,13 @@ > TokenControl->TokenSource = Token->TokenSource; > > /* Lock the token */ > - KeEnterCriticalRegion(); > - ExAcquireResourceSharedLite(Token->TokenLock, TRUE); > + SepAcquireTokenLockShared(Token); > > /* Capture the modified it */ > TokenControl->ModifiedId = Token->ModifiedId; > > /* Unlock it */ > - ExReleaseResourceLite(Token->TokenLock); > - KeLeaveCriticalRegion(); > + SepReleaseTokenLock(Token); > } > > NTSTATUS > @@ -327,13 +317,11 @@ > ClientToken = SubjectContext->ClientToken; > > /* Always lock the primary */ > - KeEnterCriticalRegion(); > - ExAcquireResourceSharedLite(PrimaryToken->TokenLock, TRUE); > + SepAcquireTokenLockShared(PrimaryToken); > > /* Lock the impersonation one if it's there */ > if (!ClientToken) return; > - KeEnterCriticalRegion(); > - ExAcquireResourceSharedLite(ClientToken->TokenLock, TRUE); > + SepAcquireTokenLockShared(ClientToken); > } > > /* > @@ -351,13 +339,11 @@ > ClientToken = SubjectContext->ClientToken; > > /* Always unlock the primary one */ > - ExReleaseResourceLite(PrimaryToken->TokenLock); > - KeLeaveCriticalRegion(); > + SepReleaseTokenLock(PrimaryToken); > > /* Unlock the impersonation one if it's there */ > if (!ClientToken) return; > - ExReleaseResourceLite(ClientToken->TokenLock); > - KeLeaveCriticalRegion(); > + SepReleaseTokenLock(ClientToken); > } > > /* > > Modified: trunk/reactos/ntoskrnl/se/semgr.c > URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/se/semgr.c?rev=57… > ============================================================================== > --- trunk/reactos/ntoskrnl/se/semgr.c [iso-8859-1] (original) > +++ trunk/reactos/ntoskrnl/se/semgr.c [iso-8859-1] Wed Sep 12 16:29:28 2012 > @@ -952,8 +952,7 @@ > SeCaptureSubjectContext(&SubjectSecurityContext); > > /* Lock the token */ > - KeEnterCriticalRegion(); > - ExAcquireResourceSharedLite(Token->TokenLock, TRUE); > + SepAcquireTokenLockShared(Token); > > /* Check if the token is the owner and grant WRITE_DAC and READ_CONTROL rights */ > if (DesiredAccess & (WRITE_DAC | READ_CONTROL | MAXIMUM_ALLOWED)) > @@ -990,8 +989,7 @@ > > /* Release subject context and unlock the token */ > SeReleaseSubjectContext(&SubjectSecurityContext); > - ExReleaseResourceLite(Token->TokenLock); > - KeLeaveCriticalRegion(); > + SepReleaseTokenLock(Token); > > /* Release the captured security descriptor */ > SeReleaseSecurityDescriptor(CapturedSecurityDescriptor, > > >

12 years, 10 months

by Ged Murphy

Why the removal of the do/while loop? This breaks the macro when used without braces : +#define SepReleaseTokenLock(Token) \ + ExReleaseResource(((PTOKEN)Token)->TokenLock); \ + KeLeaveCriticalRegion(); \ + <snip> -#define SepReleaseTokenLock(Token) \ - do { \ - ExReleaseResource(((PTOKEN)Token)->TokenLock); \ - KeLeaveCriticalRegion(); \ - while(0) <snip> /* Release the lock if we had acquired it */ + if (!TokenLocked) SepReleaseTokenLock(Token);

12 years, 10 months

Jira Planning

by Timo Kreuzer

Hi, I'd like to invite every developer to make use of the Jira planning feature. For those, who are not yet familiar with it here's a an explantion. In the blue Jira menu bar you'll find an item called "Agile". Click on that to get to the planning boards. First time users will be asked to either choose between Scrum or Kanban. Select Scrum. Then you will be asked to select a dashboard or create one. You can simply choose "Scrum Board" from the dropdown list, this one should be fine. You can later create your own board, if you like. Now you will see 3 buttons on the right-hand side below the menu bar: "plan", "work" and "report" First select "plan" On the top you can see the current "sprint", which is the next planned period. We currently have one sprint for September. We can create multiple sprints and periods are as we want them. I think one month is reasonable. Under the sprint you can see the issues that are selected for this sprint, i.e. supposed to be tackled in that period. Currently only project administrators can create sprints, but every developer can move items there. To remove an item from the sprint, you need to click on it, so it gets highlited, then open the menu with the gear wheel icon in the issue description on the right and select "Remove from sprint". Below you see the backlog (all open issues). Since the list is quite huge, you can limit it to your own issues, by pressing the "Only my issues" filter button. (Note this one works like a pushbutton, click it once to enable, again to disable. Maybe Amine can make them look more like that by adding a highlite color) On the righ-hand side, you see a box that shows the currently selected issue. Now you can drag and drop issues from the backlog into one of the sprints. So if you plan to work on a specific issue in September, just drag it into sprint 1. Now select "work" Here you can see the so called "swim lanes". In the default Scrum Board they are configured per Developer. So you see every developer that has active issues in the current sprint(s). You can open and close the swim lanes to see the issues. And you can again select "Only my issues". The board has 3 columns: "To do", "In Progress", "Done". All issues that you selected for the current sprint will initially be in the "To do" column. When you work on an issue, simply drag it over to the "In Progress" column. When you fixed it, drag it to "Done". When dragging them the target column will be highlited in green. The "Done" column will show you 2 fields when dragging an item there: one for resolve, one for close. When you dragged them to close, you'll see a confirmation dialog, where you can enter additional stuff and then confirm it. Have fun Timo

12 years, 10 months

Re: [ros-dev] [ros-diffs] [mnordell] 57198: Silence compiler warning

by Thomas Faber

Hi, On 2012-08-29 14:28, mnordell(a)svn.reactos.org wrote: > --- trunk/reactos/subsystems/csr/csrsrv/process.c [iso-8859-1] (original) > +++ trunk/reactos/subsystems/csr/csrsrv/process.c [iso-8859-1] Wed Aug 29 12:28:35 2012 > @@ -1211,7 +1211,7 @@ > BOOLEAN FirstTry; > ULONG i; > PCSR_SERVER_DLL ServerDll; > - ULONG Result; > + ULONG Result = 0; > > /* Acquire process lock */ > CsrAcquireProcessLock(); > > zero doesn't look like a valid value for this variable. Shouldn't it be one of CsrShutdownCsrProcess, CsrShutdownNonCsrProcess, CsrShutdownCancelled? -Thomas

12 years, 10 months

Re: [ros-dev] [ros-diffs] [akhaldi] 57260: [CMAKE] * Refer to the PSEH lib using its variable.

by Timo Kreuzer

Can't we get rid of that completely? Afaicr it expands to pseh for both gcc and msvc Am 10.09.2012 01:59, schrieb akhaldi(a)svn.reactos.org: > Author: akhaldi > Date: Sun Sep 9 23:59:16 2012 > New Revision: 57260 > > URL: http://svn.reactos.org/svn/reactos?rev=57260&view=rev > Log: > [CMAKE] > * Refer to the PSEH lib using its variable. > > Modified: > trunk/reactos/base/services/eventlog/CMakeLists.txt > trunk/reactos/base/services/wlansvc/CMakeLists.txt > trunk/reactos/base/system/services/CMakeLists.txt > trunk/reactos/base/system/smss/CMakeLists.txt > > Modified: trunk/reactos/base/services/eventlog/CMakeLists.txt > URL: http://svn.reactos.org/svn/reactos/trunk/reactos/base/services/eventlog/CMa… > ============================================================================== > --- trunk/reactos/base/services/eventlog/CMakeLists.txt [iso-8859-1] (original) > +++ trunk/reactos/base/services/eventlog/CMakeLists.txt [iso-8859-1] Sun Sep 9 23:59:16 2012 > @@ -17,7 +17,7 @@ > add_executable(eventlog ${SOURCE}) > > if(NOT MSVC) > - target_link_libraries(eventlog pseh) > + target_link_libraries(eventlog ${PSEH_LIB}) > endif() > > set_module_type(eventlog win32cui UNICODE) > > Modified: trunk/reactos/base/services/wlansvc/CMakeLists.txt > URL: http://svn.reactos.org/svn/reactos/trunk/reactos/base/services/wlansvc/CMak… > ============================================================================== > --- trunk/reactos/base/services/wlansvc/CMakeLists.txt [iso-8859-1] (original) > +++ trunk/reactos/base/services/wlansvc/CMakeLists.txt [iso-8859-1] Sun Sep 9 23:59:16 2012 > @@ -10,7 +10,7 @@ > ${CMAKE_CURRENT_BINARY_DIR}/wlansvc_s.c) > > if(NOT MSVC) > - target_link_libraries(wlansvc pseh) > + target_link_libraries(wlansvc ${PSEH_LIB}) > endif() > > set_module_type(wlansvc win32cui UNICODE) > > Modified: trunk/reactos/base/system/services/CMakeLists.txt > URL: http://svn.reactos.org/svn/reactos/trunk/reactos/base/system/services/CMake… > ============================================================================== > --- trunk/reactos/base/system/services/CMakeLists.txt [iso-8859-1] (original) > +++ trunk/reactos/base/system/services/CMakeLists.txt [iso-8859-1] Sun Sep 9 23:59:16 2012 > @@ -22,7 +22,7 @@ > add_executable(services ${SOURCE}) > > if(NOT MSVC) > - target_link_libraries(services pseh) > + target_link_libraries(services ${PSEH_LIB}) > endif() > > set_module_type(services win32cui UNICODE) > > Modified: trunk/reactos/base/system/smss/CMakeLists.txt > URL: http://svn.reactos.org/svn/reactos/trunk/reactos/base/system/smss/CMakeList… > ============================================================================== > --- trunk/reactos/base/system/smss/CMakeLists.txt [iso-8859-1] (original) > +++ trunk/reactos/base/system/smss/CMakeLists.txt [iso-8859-1] Sun Sep 9 23:59:16 2012 > @@ -2,23 +2,20 @@ > include_directories(${REACTOS_SOURCE_DIR}/include/reactos/subsys) > > list(APPEND SOURCE > - crashdmp.c > - pagefile.c > - sminit.c > - smloop.c > - smsbapi.c > - smsessn.c > - smsubsys.c > - smutil.c > - smss.c > - smss.rc) > + crashdmp.c > + pagefile.c > + sminit.c > + smloop.c > + smsbapi.c > + smsessn.c > + smsubsys.c > + smutil.c > + smss.c > + smss.rc) > > add_executable(smss WIN32 ${SOURCE}) > - > -target_link_libraries(smss nt pseh smlib) > - > +target_link_libraries(smss nt ${PSEH_LIB} smlib) > add_pch(smss smss.h) > - > set_module_type(smss nativecui) > add_importlibs(smss ntdll) > add_cd_file(TARGET smss DESTINATION reactos/system32 FOR all) > > >

12 years, 10 months

PSEH3

by Timo Kreuzer

Hi, I am pleased to announce the long awaited (or absolutely surprising) release of PSEH3. This complete from scratch rewrite takes reactos exception handling to a new level. It comprises the following advancements: - Use of enums instead of const variables for internal trylevel counting and other mechanisms. enums can be declared in a nested manner as well and only enums are treated by gcc as full compile time constants, while const variables are treated as dynamic and are only optimized away by the compiler later. This generally results in better optimized code. - No more use of dynamic sized arrays. Those make the compiler emit a slow call to _chkstk. It could have been prevented in PSEH2, if enums had been used instead of consts. The new code uses the same registration frame for the top level and nested levels, which also simplifies the code. - No more use of nested function stack trampolines. These are small code chunks that are being dynamically put on the stack, when the address of a nested function is taken. The address of the nested function then points to this trampoline, which sets up a pointer for nested variable access (nested function can access variables in the parent frame). These were parsed by PSEH2 to calculate the neccessary nested frame pointer. PSEH3 uses a static scope table instead that contains the address of the nested function. this is only possible by first declaring the function and putting the implementation after the address is stored in the static table. This is because nested functions that do not access any variables in the parent frame do not need a stack trampoline and their addresses are thus considered compiler constants. GCC doesn't notice that a nested function uses variables of the parent frame, before the function is actually implemented, so with only a prototype, it assumes a constant address. To get the value of the parent frame pointer, PSEH2 calls the nested function 2 times: The first time with a bogus frame pointer and a parameter that tells the function to return the (assumed) address of the current registration frame. Without a proper parent frame pointer the nested function will return an offset that we can use together with the real address of the registration frame (which we know, when calling the filter of finally functions) to calculate the proper parent frame pointer. The 2nd call passes a different parameter and allows the function to do the actual work and access variables in the parent frame. - Improved performance due to use of "asm goto". This construct allows to get away with having labels, that are never accessed by any C code. The compiler doesn't remove them. So we do not need the old hack, of comparing a "volatile" 0 against 0, just to confuse GCC to not optimize away a label. Even though there is a bug/defficiency/limitation in asm goto, that would usually not allow the combination with static C jump labels, we can still do that due to some tricks that force GCC to do the right thing. More information: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51840 - Improved performance due to faster registration. The new registration code is smaller and saves only esp and ebp. Due to some tricks in PSEH3, GCC is forced to assume all registers are being clobbered when the __except block is run. Filter function and __finally function don't make use of registers from the previous code. So all neccessary register save and restore is now fully done by GCC. - The trick to make GCC assume all registers are clobbered at the beginning of the __except block has one additional huge advantage. Now all variables that are accessed by the __except block will be stored in memory locations, so that modifications in the __try block will be available in the __except block, something that doesn't work with PSEH2. This does not prevent the compiler from doing constant expression elimination! So when you try to do 2 different, constant assignments to a variable in the __try block, expect that this will be reduced to only the later assignment. GCC does not know, that the execution can be interrupted in the middle of the __try block between those 2 assignments. If you need such stuff, you must use a volatile variable. - Less stack usage. PSEH3 stores much less data on the stack. No trampolines, only esp and ebp are saved, registration frames are smaller. - No more need for _SEH2_YIELD(). This macro was used to make sure the registration frames are properly unregistered, when leaving the try or except block using goto or return. This is not neccessary anymore due to the use of __attribute__((cleanup)). This attribute is attached to the registration frame and causes the cleanup function to be called automatically, whenever the variable goes out of scope. This happens when a goto or return is used in the code. - Human readable warnings, when special SEH instriniscs, like _exception_info() or _abnormal_termination() are used outside of their valid context. - Cleaner and better readable code, so you won't get braindead, when trying to figure out what's going on. Header: < 300 lines commented code, instead of 370 lines uncommented code, lib: < 250 lines of commented C code + < 70 lines commented asm code, instead of 290 lines of uncommented C code + 120 lines uncommented asm code) I also promised Amine to take a look into implementing a version that is compatible with clang. Initially I planned to include this into this release, but it turns out that clang massivley lacks features to do so and a completely different approach will probably be used for that. The new code is only compatible with GCC 4.7 and later, and it's also currently disabled (cmake option USE_PSEH3). Tesbot showed no noticable regressions. Have fun. Timo

12 years, 10 months

Bugzilla outage

by Aleksey Bragin

Hello, I turned off Bugzilla now in favor of finishing the switch to JIRA. Once the work is done, I will announce either return to Bugzilla or introduce JIRA. Hopefully, it'll be the latter option. Best regards, Aleksey.

12 years, 11 months

Fwd: buildbot failure in ReactOS on CMake_x86_MSVCWin Debug

by caemyr＠myopera.com

This commit breaks msvc compilation. Best regards ----- Original message ----- From: buildbot(a)reactos.org To: caemyr(a)myopera.com Subject: buildbot failure in ReactOS on CMake_x86_MSVCWin Debug Date: Tue, 04 Sep 2012 03:10:09 +0000 The Buildbot has finished a build on builder CMake_x86_MSVCWin Debug while building ReactOS. Full details are available at: http://build.reactos.org/builders/CMake_x86_MSVCWin%20Debug/builds/1908 Buildbot URL: http://build.reactos.org/ Buildslave for this Build: Windows_AMD64_1 Build Reason: scheduler Build Source Stamp: 57233 Blamelist: cgutman BUILD FAILED: failed building bootcd sincerely, -The Buildbot -- With best regards Caemyr

12 years, 11 months

Re: [ros-dev] buildbot failure in ReactOS on Windows_AMD64_1 VBox-Test

by caemyr＠myopera.com

Currently, we experience random crashes in 2nd stage. I was unable to catch any first exception bar the one with vmware detection code (which is outdated and should be removed). After forcing build again, test run failed at Cleanup, due to previously unresolved issue of repeating whole testrun when crash occurs in APItests. On Mon, Sep 3, 2012, at 10:21 AM, buildbot(a)reactos.org wrote: > The Buildbot has finished a build on builder Windows_AMD64_1 VBox-Test > while building ReactOS. > Full details are available at: > http://build.reactos.org/builders/Windows_AMD64_1%20VBox-Test/builds/6496 > > Buildbot URL: http://build.reactos.org/ > > Buildslave for this Build: Windows_AMD64_2 > > Build Reason: The web-page 'force build' button was pressed by 'osiejka': > > Build Source Stamp: 57227 > Blamelist: > > BUILD FAILED: failed Cleanup > > sincerely, > -The Buildbot > -- With best regards Caemyr

12 years, 11 months

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

2004

Ros-dev September 2012