Ros-dev October 2014

ros-dev@reactos.org

15 participants
38 discussions

Tuesday is a best day for software\pr releases

by Alexander Rechitskiy

10 years, 2 months

Re: [ros-dev] [ros-diffs] [jgardou] 64911: [USER32] - Fix LookupIconIdFromDirectoryEx, returning 0 when no matching entry is found. - Fix error handling when opening a cursor file. - Various code beautification here and the...

by Thomas Faber

On 2014-10-23 11:32, jgardou(a)svn.reactos.org wrote: > - if ( dwFileSize < (sizeof(*dir) + sizeof(dir->idEntries[0])*(dir->idCount-1)) ) > + if (dwFileSize < (sizeof(*dir) + FIELD_OFFSET(CURSORICONFILEDIR, idEntries[dir->idCount]))) Did you mean to remove the sizeof(*dir)?

10 years, 2 months

Re: [ros-dev] [ros-diffs] [tfaber] 64889: [NTOS:MM] - Add a way to generate a pool tag from the calling driver name if none is specified. Disabled by default.

by Timo Kreuzer

Simpler: + if (LdrEntry) + { + ULONG i; + Tag = '____'; // IMO better than ' ' + for (i = 0; i < min(4, LdrEntry->BaseDllName.Length / sizeof(WCHAR)); i++) + ((PCHAR)&Tag)[i] = (LdrEntry->BaseDllName.Buffer[i] & 0xff); + } Maybe we can also add support for tracking backtraces of pool allocations. Am 22.10.2014 15:26, schrieb tfaber(a)svn.reactos.org: > Author: tfaber > Date: Wed Oct 22 13:26:50 2014 > New Revision: 64889 > > URL: http://svn.reactos.org/svn/reactos?rev=64889&view=rev > Log: > [NTOS:MM] > - Add a way to generate a pool tag from the calling driver name if none is specified. Disabled by default. > > Modified: > trunk/reactos/ntoskrnl/mm/ARM3/expool.c > > Modified: trunk/reactos/ntoskrnl/mm/ARM3/expool.c > URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/ARM3/expool.c?… > ============================================================================== > --- trunk/reactos/ntoskrnl/mm/ARM3/expool.c [iso-8859-1] (original) > +++ trunk/reactos/ntoskrnl/mm/ARM3/expool.c [iso-8859-1] Wed Oct 22 13:26:50 2014 > @@ -2046,10 +2046,25 @@ > ExAllocatePool(POOL_TYPE PoolType, > SIZE_T NumberOfBytes) > { > - // > - // Use a default tag of "None" > - // > - return ExAllocatePoolWithTag(PoolType, NumberOfBytes, TAG_NONE); > + ULONG Tag = TAG_NONE; > +#if 0 && DBG > + PLDR_DATA_TABLE_ENTRY LdrEntry; > + > + /* Use the first four letters of the driver name, or "None" if unavailable */ > + LdrEntry = KeGetCurrentIrql() <= APC_LEVEL > + ? MiLookupDataTableEntry(_ReturnAddress()) > + : NULL; > + if (LdrEntry) > + { > + ULONG i; > + Tag = 0; > + for (i = 0; i < min(4, LdrEntry->BaseDllName.Length / sizeof(WCHAR)); i++) > + Tag = Tag >> 8 | (LdrEntry->BaseDllName.Buffer[i] & 0xff) << 24; > + for (; i < 4; i++) > + Tag = Tag >> 8 | ' ' << 24; > + } > +#endif > + return ExAllocatePoolWithTag(PoolType, NumberOfBytes, Tag); > } > > /* > @@ -2513,7 +2528,7 @@ > // > // Allocate the pool > // > - return ExAllocatePoolWithQuotaTag(PoolType, NumberOfBytes, 'enoN'); > + return ExAllocatePoolWithQuotaTag(PoolType, NumberOfBytes, TAG_NONE); > } > > /* > > >

10 years, 2 months

Re: [ros-dev] [ros-diffs] [khornicek] 64882: [SERVMAN] - fix resource leaks CID 716292, 716293, 716294 - fix CID 716772 (double free), 513719 (wrong NULL check), 1206739 (cosmetic) - make line endings CR LF when exporting t...

by Timo Kreuzer

Am 22.10.2014 00:58, schrieb khornicek(a)svn.reactos.org: > - if (LVText != NULL) > + if (_tcslen(LVText)) > { > WriteFile(hFile, > LVText, I think LVText might not be zero terminated, when the SendMessage call fails. What about if (dwTextLength != 0)?

10 years, 2 months

Re: [ros-dev] [ros-diffs] [hbelusca] 64773: [FAST486]: Do not call RtlCopyMemory for copying few bytes (2 and 4).

by Alex Ionescu

memcpy already has logic to handle, 1, 2, 4, 8, etc.. sizes. Best regards, Alex Ionescu On Thu, Oct 16, 2014 at 2:48 PM, <hbelusca(a)svn.reactos.org> wrote: > Author: hbelusca > Date: Thu Oct 16 21:48:18 2014 > New Revision: 64773 > > URL: http://svn.reactos.org/svn/reactos?rev=64773&view=rev > Log: > [FAST486]: Do not call RtlCopyMemory for copying few bytes (2 and 4). > > Modified: > trunk/reactos/lib/fast486/fast486.c > trunk/reactos/lib/fast486/opgroups.c > > Modified: trunk/reactos/lib/fast486/fast486.c > URL: > http://svn.reactos.org/svn/reactos/trunk/reactos/lib/fast486/fast486.c?rev=… > > ============================================================================== > --- trunk/reactos/lib/fast486/fast486.c [iso-8859-1] (original) > +++ trunk/reactos/lib/fast486/fast486.c [iso-8859-1] Thu Oct 16 21:48:18 > 2014 > @@ -117,7 +117,6 @@ > Fast486MemReadCallback(PFAST486_STATE State, ULONG Address, PVOID Buffer, > ULONG Size) > { > UNREFERENCED_PARAMETER(State); > - > RtlMoveMemory(Buffer, (PVOID)Address, Size); > } > > @@ -126,7 +125,6 @@ > Fast486MemWriteCallback(PFAST486_STATE State, ULONG Address, PVOID > Buffer, ULONG Size) > { > UNREFERENCED_PARAMETER(State); > - > RtlMoveMemory((PVOID)Address, Buffer, Size); > } > > > Modified: trunk/reactos/lib/fast486/opgroups.c > URL: > http://svn.reactos.org/svn/reactos/trunk/reactos/lib/fast486/opgroups.c?rev… > > ============================================================================== > --- trunk/reactos/lib/fast486/opgroups.c [iso-8859-1] (original) > +++ trunk/reactos/lib/fast486/opgroups.c [iso-8859-1] Thu Oct 16 > 21:48:18 2014 > @@ -2016,6 +2016,7 @@ > > FAST486_OPCODE_HANDLER(Fast486OpcodeGroup0F01) > { > + // FAST486_TABLE_REG TableReg; > UCHAR TableReg[6]; > FAST486_MOD_REG_RM ModRegRm; > BOOLEAN OperandSize, AddressSize; > @@ -2054,8 +2055,9 @@ > } > > /* Fill the 6-byte table register */ > - RtlCopyMemory(TableReg, &State->Gdtr.Size, sizeof(USHORT)); > - RtlCopyMemory(&TableReg[sizeof(USHORT)], > &State->Gdtr.Address, sizeof(ULONG)); > + // TableReg = State->Gdtr; > + *((PUSHORT)&TableReg) = State->Gdtr.Size; > + *((PULONG)&TableReg[sizeof(USHORT)]) = State->Gdtr.Address; > > /* Store the GDTR */ > return Fast486WriteMemory(State, > @@ -2076,8 +2078,9 @@ > } > > /* Fill the 6-byte table register */ > - RtlCopyMemory(TableReg, &State->Idtr.Size, sizeof(USHORT)); > - RtlCopyMemory(&TableReg[sizeof(USHORT)], > &State->Idtr.Address, sizeof(ULONG)); > + // TableReg = State->Idtr; > + *((PUSHORT)&TableReg) = State->Idtr.Size; > + *((PULONG)&TableReg[sizeof(USHORT)]) = State->Idtr.Address; > > /* Store the IDTR */ > return Fast486WriteMemory(State, > @@ -2117,7 +2120,8 @@ > } > > /* Load the new GDT */ > - State->Gdtr.Size = *((PUSHORT)TableReg); > + // State->Gdtr = TableReg; > + State->Gdtr.Size = *((PUSHORT)&TableReg); > State->Gdtr.Address = *((PULONG)&TableReg[sizeof(USHORT)]); > > /* In 16-bit mode the highest byte is masked out */ > @@ -2156,7 +2160,8 @@ > } > > /* Load the new IDT */ > - State->Idtr.Size = *((PUSHORT)TableReg); > + // State->Idtr = TableReg; > + State->Idtr.Size = *((PUSHORT)&TableReg); > State->Idtr.Address = *((PULONG)&TableReg[sizeof(USHORT)]); > > /* In 16-bit mode the highest byte is masked out */ > > >

10 years, 2 months

Re: [ros-dev] [ros-diffs] [hbelusca] 64747: [NTVDM]: Zero-fill memory with RtlZeroMemory (that exists also in NT mode), and use sizeof(object) instead of sizeof(type_of_object).

by Timo Kreuzer

Looks like bugs Am 15.10.2014 00:46, schrieb hbelusca(a)svn.reactos.org: > Author: hbelusca > Date: Tue Oct 14 22:46:40 2014 > New Revision: 64747 > > URL: http://svn.reactos.org/svn/reactos?rev=64747&view=rev > Log: > [NTVDM]: Zero-fill memory with RtlZeroMemory (that exists also in NT mode), and use sizeof(object) instead of sizeof(type_of_object). > /* Clear the current directory buffer */ > - ZeroMemory(CurrentDirectories, sizeof(CurrentDirectories)); > + RtlZeroMemory(CurrentDirectories, sizeof(CurrentDirectories)); > @@ -2901,7 +2901,7 @@ > WCHAR Buffer[256]; > > /* Clear the current directory buffer */ > - ZeroMemory(CurrentDirectories, sizeof(CurrentDirectories)); > + RtlZeroMemory(CurrentDirectories, sizeof(CurrentDirectories)); > @@ -1901,7 +1901,7 @@ > > VOID VgaClearMemory(VOID) > { > - ZeroMemory(VgaMemory, sizeof(VgaMemory)); > + RtlZeroMemory(VgaMemory, sizeof(VgaMemory)); > } > >

10 years, 2 months

Re: [ros-dev] RtlUlonglongByteSwap (Thomas Faber)

by Love Nystrom

On 2014-10-17 19.00, ros-dev-request(a)reactos.org wrote: > Fastcall passes 64 bit arguments on the stack. Aaagh.. I didn't know that.. Thanks for pointing it out. IMO that's incredibly stupid, as a 32-bit CPU (or x64 running 32bit) uses the register combination EAX:EDX for 64 bit integers, and EAX:EDX is used for 64 bit return values (again in 32bit mode). The compiler designers need to smarten up! Best Regards Love > And yes, this is about an ntdll/ntoskrnl export, so we must be binary > compatible with what Windows does. > But the implementation that is normally used is an inline/intrinsic > function anyway. This is just about the export (or rather, importing > it). > > On 2014-10-17 00:24, Love Nystrom wrote: >> >@Timo, @Thomas, @Ged >> > >> >Byte order swappers should always be fastcall for perfomance reasons. >> >They have no need for the benefits of the cdecl call convention. >> >Using cdecl in this case would make the binary code pitifully slow. >> > >> >Think about it for a bit.. Some pseudocode show what I mean: >> > >> >..CDECL... >> > >> > push hi >> > push lo >> > call Swapper >> > mov dsthi, edx >> > mov dstlo, eax >> > add esp, 4 >> > ... >> > >> >// UINT64 __cdecl >> >Swapper: >> > push ebp >> > mov ebp, esp >> > mov eax, ebp+8 // lo >> > mov eax, ebp+12 // hi >> > bswap >> > xchg eax, edx >> > bswap >> > pop ebp >> > ret >> > >> >..FASTCALL... >> > >> > mov edx, hi >> > mov eax, lo >> > call Swapper >> > mov dsthi, edx >> > mov dstlo, eax >> > ... >> > >> >// UINT64 __declspec(naked) __fastcall >> >Swapper: >> > bswap >> > xchg eax, edx >> > bswap >> > ret >> > >> >Sadly the compiler designers were not (yet) clever enough >> >to make the fastcall regs EAX, EDX, ECX, in that exact order, >> >but even as it stands today.. >> > >> >Swapper: >> > mov eax, ecx >> > bswap >> > xchg eax, edx >> > bswap >> > ret >> > >> > >> >(If you actually link against a binary swapper compiled out of your >> >control with >> >cdecl convention, the argument falls of course, as you must comply with >> >the binary.) >> > >> >Keep up the good work.. >> >Best Regards >> >// Love >

10 years, 2 months

Video of our ReactOS presentation online

by Colin Finck

Hi all, As the question usually arises: The video of Daniel's and my ReactOS presentation at Kieler Linux Tage is online: http://youtu.be/oQsKzW_Rx3k Do whatever you want with it ;) This is probably the first ever ReactOS presentation held using ReactOS itself! And it worked like a charm! We also met an author of German news site heise.de there, who wants to publish an article about the current state ReactOS soon. Cheers, Colin

10 years, 2 months

Re: [ros-dev] [ros-diffs] [akhaldi] 64795: [CMAKE] * Make the minimum required version 2.8. * Remove redundant psdk dependencies. * Tidy up CMake files.

by Thomas Faber

On 2014-10-18 01:28, akhaldi(a)svn.reactos.org wrote: > [CMAKE] > * Make the minimum required version 2.8. As Víctor just found out the hard way, using target_include_directories makes our minimum version 2.8.11. Should we set that as the minimum to create a more obvious error message? Current error: CMake Error at dll/keyboard/CMakeLists.txt:92 (target_include_directories): Unknown CMake command "target_include_directories".

10 years, 2 months

Re: [ros-dev] RtlUlonglongByteSwap

by Love Nystrom

@Timo, @Thomas, @Ged Byte order swappers should always be fastcall for perfomance reasons. They have no need for the benefits of the cdecl call convention. Using cdecl in this case would make the binary code pitifully slow. Think about it for a bit.. Some pseudocode show what I mean: ..CDECL... push hi push lo call Swapper mov dsthi, edx mov dstlo, eax add esp, 4 ... // UINT64 __cdecl Swapper: push ebp mov ebp, esp mov eax, ebp+8 // lo mov eax, ebp+12 // hi bswap xchg eax, edx bswap pop ebp ret ..FASTCALL... mov edx, hi mov eax, lo call Swapper mov dsthi, edx mov dstlo, eax ... // UINT64 __declspec(naked) __fastcall Swapper: bswap xchg eax, edx bswap ret Sadly the compiler designers were not (yet) clever enough to make the fastcall regs EAX, EDX, ECX, in that exact order, but even as it stands today.. Swapper: mov eax, ecx bswap xchg eax, edx bswap ret (If you actually link against a binary swapper compiled out of your control with cdecl convention, the argument falls of course, as you must comply with the binary.) Keep up the good work.. Best Regards // Love

10 years, 2 months

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

2004

Ros-dev October 2014