Ros-dev May 2017

ros-dev@reactos.org

17 participants
26 discussions

by Aleksey Bragin

Hello, I propose to reschedule our monthly meeting to Thursday next week (1st of June), as tomorrow is a holiday in some countries, and also I have tight meetings schedule this week. Regards, Aleksey Bragin

7 years, 5 months

Re: [ros-dev] [ros-diffs] [hbelusca] 74698: [USETUP]: Minor fixes & simplifications: - Remove a redundant NtClose call; - Return failure if NtQuerySymbolicLinkObject fails; - Use RTL_CONSTANT_STRING and RtlInitEmptyUnicodeS...

by Colin Finck

hbelusca(a)svn.reactos.org wrote: > -/* > - * ReactOS kernel > - * Copyright (C) 2002 ReactOS Team > - * > - * This program is free software; you can redistribute it and/or modify > - * it under the terms of the GNU General Public License as published by > - * the Free Software Foundation; either version 2 of the License, or > - * (at your option) any later version. > - * > - * This program is distributed in the hope that it will be useful, > - * but WITHOUT ANY WARRANTY; without even the implied warranty of > - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > - * GNU General Public License for more details. > - * > - * You should have received a copy of the GNU General Public License along > - * with this program; if not, write to the Free Software Foundation, Inc., > - * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. > - */ > /* > * COPYRIGHT: See COPYING in the top level directory > * PROJECT: ReactOS text-mode setup You're taking away the information that this file is licensed under GPLv2 or any later version (as opposed to GPLv2-only, what some developers prefer). Also the entire license information is lost when this file is used as part of a different source tree. I have visited a seminar by our friend Nuno Brito last week, who has founded a company that deals with such topics (www.triplecheck.tech). There are some simple best practices we can apply here to get consistent and useful headers once and for all. The first would be using the established SPDX license identifiers from https://spdx.org/licenses (here: "GPL-2.0+"). This would also make our source files easily scannable by tools to get an overview of all licenses involved in our code and imported third-party components. If you or someone else is interested in working on this, let's draft a universal header for our files that is compatible with SPDX. Otherwise, I will have a look when there's more time.. Cheers, Colin

7 years, 5 months

Re: [ros-dev] [ros-diffs] [mjansen] 74693: [KERNEL32] Initial implementation for BaseCheckRunApp, calling into apphelp to check for compatibility fixes CORE-10368

by Thomas Faber

Testbots seem to dislike this commit, looks like 3rd stage boot is broken in some way. :( On 2017-05-28 21:27, mjansen(a)svn.reactos.org wrote: > Author: mjansen > Date: Sun May 28 19:27:51 2017 > New Revision: 74693 > > URL: http://svn.reactos.org/svn/reactos?rev=74693&view=rev > Log: > [KERNEL32] Initial implementation for BaseCheckRunApp, calling into apphelp to check for compatibility fixes CORE-10368 > > Modified: > trunk/reactos/dll/win32/kernel32/client/appcache.c > trunk/reactos/dll/win32/kernel32/client/utils.c > > Modified: trunk/reactos/dll/win32/kernel32/client/appcache.c > URL: http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/kernel32/client/… > ============================================================================== > --- trunk/reactos/dll/win32/kernel32/client/appcache.c [iso-8859-1] (original) > +++ trunk/reactos/dll/win32/kernel32/client/appcache.c [iso-8859-1] Sun May 28 19:27:51 2017 > @@ -16,7 +16,20 @@ > /* GLOBALS ********************************************************************/ > > ULONG g_ShimsEnabled; > - > +static BOOL g_ApphelpInitialized = FALSE; > +static PVOID g_pApphelpCheckRunAppEx; > +static PVOID g_pSdbPackAppCompatData; > + > +typedef BOOL (WINAPI *tApphelpCheckRunAppEx)(HANDLE FileHandle, PVOID Unk1, PVOID Unk2, PWCHAR ApplicationName, PVOID Environment, USHORT ExeType, PULONG Reason, > + PVOID* SdbQueryAppCompatData, PULONG SdbQueryAppCompatDataSize, PVOID* SxsData, PULONG SxsDataSize, > + PULONG FusionFlags, PULONG64 SomeFlag1, PULONG SomeFlag2); > +typedef BOOL (WINAPI *tSdbPackAppCompatData)(PVOID hsdb, PVOID pQueryResult, PVOID* ppData, DWORD *dwSize); > + > +#define APPHELP_VALID_RESULT 0x10000 > +#define APPHELP_RESULT_NOTFOUND 0x20000 > +#define APPHELP_RESULT_FOUND 0x40000 > + > + > /* FUNCTIONS ******************************************************************/ > > BOOLEAN > @@ -136,7 +149,120 @@ > DPRINT("BaseCheckAppcompatCache is UNIMPLEMENTED\n"); > > if (Reason) *Reason = 0; > - return TRUE; > + > + // We don't know this app. > + return FALSE; > +} > + > +static > +VOID > +BaseInitApphelp(VOID) > +{ > + WCHAR Buffer[MAX_PATH*2]; > + UNICODE_STRING DllPath = {0}; > + PVOID ApphelpAddress; > + PVOID pApphelpCheckRunAppEx = NULL, pSdbPackAppCompatData = NULL; > + > + RtlInitEmptyUnicodeString(&DllPath, Buffer, sizeof(Buffer)); > + RtlCopyUnicodeString(&DllPath, &BaseWindowsDirectory); > + RtlAppendUnicodeToString(&DllPath, L"\\system32\\apphelp.dll"); > + > + if (NT_SUCCESS(LdrLoadDll(NULL, NULL, &DllPath, &ApphelpAddress))) > + { > + ANSI_STRING ProcName; > + > + RtlInitAnsiString(&ProcName, "ApphelpCheckRunAppEx"); > + if (!NT_SUCCESS(LdrGetProcedureAddress(ApphelpAddress, &ProcName, 0, &pApphelpCheckRunAppEx))) > + pApphelpCheckRunAppEx = NULL; > + > + RtlInitAnsiString(&ProcName, "SdbPackAppCompatData"); > + if (!NT_SUCCESS(LdrGetProcedureAddress(ApphelpAddress, &ProcName, 0, &pSdbPackAppCompatData))) > + pSdbPackAppCompatData = NULL; > + } > + > + if (InterlockedCompareExchangePointer(&g_pApphelpCheckRunAppEx, RtlEncodeSystemPointer(pApphelpCheckRunAppEx), NULL) == NULL) > + { > + g_pSdbPackAppCompatData = RtlEncodeSystemPointer(pSdbPackAppCompatData); > + } > +} > + > +/* > + * > + */ > +BOOL > +WINAPI > +BaseCheckRunApp(IN HANDLE FileHandle, > + IN PWCHAR ApplicationName, > + IN PWCHAR Environment, > + IN USHORT ExeType, > + IN PULONG pReason, > + IN PVOID* SdbQueryAppCompatData, > + IN PULONG SdbQueryAppCompatDataSize, > + IN PVOID* SxsData, > + IN PULONG SxsDataSize, > + OUT PULONG FusionFlags) > +{ > + ULONG Reason = 0; > + ULONG64 Flags1 = 0; > + ULONG Flags2 = 0; > + BOOL Continue, NeedCleanup = FALSE; > + tApphelpCheckRunAppEx pApphelpCheckRunAppEx; > + tSdbPackAppCompatData pSdbPackAppCompatData; > + PVOID QueryResult = NULL; > + ULONG QueryResultSize = 0; > + > + if (!g_ApphelpInitialized) > + { > + BaseInitApphelp(); > + g_ApphelpInitialized = TRUE; > + } > + > + pApphelpCheckRunAppEx = RtlDecodeSystemPointer(g_pApphelpCheckRunAppEx); > + pSdbPackAppCompatData = RtlDecodeSystemPointer(g_pSdbPackAppCompatData); > + > + if (!pApphelpCheckRunAppEx || !pSdbPackAppCompatData) > + return TRUE; > + > + if (pReason) > + Reason = *pReason; > + > + Continue = pApphelpCheckRunAppEx(FileHandle, NULL, NULL, ApplicationName, Environment, ExeType, &Reason, > + &QueryResult, &QueryResultSize, SxsData, SxsDataSize, FusionFlags, &Flags1, &Flags2); > + > + if (pReason) > + *pReason = Reason; > + > + if (Continue) > + { > + if ((Reason & (APPHELP_VALID_RESULT|APPHELP_RESULT_FOUND)) == (APPHELP_VALID_RESULT|APPHELP_RESULT_FOUND)) > + { > + if (!pSdbPackAppCompatData(NULL, QueryResult, SdbQueryAppCompatData, SdbQueryAppCompatDataSize)) > + { > + DPRINT1("SdbPackAppCompatData returned a failure!\n"); > + NeedCleanup = TRUE; > + } > + } > + else > + { > + NeedCleanup = TRUE; > + } > + } > + > + if (QueryResult) > + RtlFreeHeap(RtlGetProcessHeap(), 0, QueryResult); > + > + if (NeedCleanup) > + { > + BasepFreeAppCompatData(*SdbQueryAppCompatData, *SxsData); > + *SdbQueryAppCompatData = NULL; > + if (SdbQueryAppCompatDataSize) > + *SdbQueryAppCompatDataSize = 0; > + *SxsData = NULL; > + if (SxsDataSize) > + *SxsDataSize = 0; > + } > + > + return Continue; > } > > /* > @@ -171,9 +297,11 @@ > Environment, > &Reason)) > { > - /* We don't support this yet */ > - UNIMPLEMENTED; > - Status = STATUS_ACCESS_DENIED; > + if (!BaseCheckRunApp(FileHandle, ApplicationName, Environment, ExeType, &Reason, > + SdbQueryAppCompatData, SdbQueryAppCompatDataSize, SxsData, SxsDataSize, FusionFlags)) > + { > + Status = STATUS_ACCESS_DENIED; > + } > } > } > > > 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] Sun May 28 19:27:51 2017 > @@ -904,26 +904,6 @@ > /* > * @unimplemented > */ > -BOOL > -WINAPI > -BaseCheckRunApp(IN DWORD Unknown1, > - IN DWORD Unknown2, > - IN DWORD Unknown3, > - IN DWORD Unknown4, > - IN DWORD Unknown5, > - IN DWORD Unknown6, > - IN DWORD Unknown7, > - IN DWORD Unknown8, > - IN DWORD Unknown9, > - IN DWORD Unknown10) > -{ > - STUB; > - return FALSE; > -} > - > -/* > - * @unimplemented > - */ > NTSTATUS > WINAPI > BasepCheckWinSaferRestrictions(IN HANDLE UserToken, > >

7 years, 5 months

Re: [ros-dev] [ros-diffs] [hbelusca] 74621: [USETUP]: SetupLib: Add an ARC path to (and from) NT path resolver. It allows mapping between an ARC path as specified in freeldr.ini / boot.ini , to its corresponding NT path, if...

by Thomas Faber

On 2017-05-22 03:09, hbelusca(a)svn.reactos.org wrote: > --- branches/setup_improvements/base/setup/lib/arcname.c (added) > +++ branches/setup_improvements/base/setup/lib/arcname.c [iso-8859-1] Mon May 22 01:09:35 2017 > +PCSTR > +ArcGetNextTokenA( > + IN PCSTR ArcPath, > + OUT PANSI_STRING TokenSpecifier, > + OUT PULONG Key) > +{ > + HRESULT hr; > + PCSTR p = ArcPath; > + ULONG SpecifierLength; > + ULONG KeyValue; > + > + /* > + * We must have a valid "specifier(key)" string, where 'specifier' > + * cannot be the empty string, and is followed by '('. > + */ > + p = strchr(p, '('); > + if (p == NULL) > + return NULL; /* No '(' found */ > + if (p == ArcPath) > + return NULL; /* Path starts with '(' and is thus invalid */ > + > + SpecifierLength = (p - ArcPath) * sizeof(CHAR); > + > + /* > + * The strtoul function skips any leading whitespace. > + * > + * Note that if the token is "specifier()" then strtoul won't perform > + * any conversion and return 0, therefore effectively making the token > + * equivalent to "specifier(0)", as it should be. > + */ > + // KeyValue = atoi(p); > + KeyValue = strtoul(p, (PSTR*)&p, 10); > + > + /* Skip any trailing whitespace */ > + while (*p && isspace(*p)) ++p; isspace(0) is false so you don't need the *p check. > +PCWSTR > +ArcGetNextTokenU( > + IN PCWSTR ArcPath, > + OUT PUNICODE_STRING TokenSpecifier, > + OUT PULONG Key) > +{ > + HRESULT hr; > + PCWSTR p = ArcPath; > + ULONG SpecifierLength; > + ULONG KeyValue; > + > + /* > + * We must have a valid "specifier(key)" string, where 'specifier' > + * cannot be the empty string, and is followed by '('. > + */ > + p = wcschr(p, L'('); > + if (p == NULL) > + return NULL; /* No '(' found */ > + if (p == ArcPath) > + return NULL; /* Path starts with '(' and is thus invalid */ > + > + SpecifierLength = (p - ArcPath) * sizeof(WCHAR); > + > + ++p; > + > + /* > + * The strtoul function skips any leading whitespace. > + * > + * Note that if the token is "specifier()" then strtoul won't perform > + * any conversion and return 0, therefore effectively making the token > + * equivalent to "specifier(0)", as it should be. > + */ > + // KeyValue = _wtoi(p); > + KeyValue = wcstoul(p, (PWSTR*)&p, 10); > + ASSERT(p); This assert seems superfluous, you just dereferenced the pointer. Also, you incremented it just above and also (rightly) assumed it was larger than ArcPath. > +static NTSTATUS > +ResolveArcNameManually( > + OUT PUNICODE_STRING NtName, > + IN OUT PCWSTR* ArcNamePath, > + IN PPARTLIST PartList OPTIONAL) > +{ > [...] > +Quit: > + if (FAILED(hr)) > + { > + /* > + * We can directly cast the HRESULTs into NTSTATUS since the error codes > + * returned by StringCbPrintfW: > + * STRSAFE_E_INVALID_PARAMETER == 0x80070057, > + * STRSAFE_E_INSUFFICIENT_BUFFER == 0x8007007a, > + * do not have assigned values in the NTSTATUS space. > + */ > + return (NTSTATUS)hr; To me that sounds like an argument of why you _can't_ do that. (You know the Rtl versions of these functions give you an NTSTATUS btw, right?) > --- branches/setup_improvements/base/setup/lib/arcname_tests.c (added) > +++ branches/setup_improvements/base/setup/lib/arcname_tests.c [iso-8859-1] Mon May 22 01:09:35 2017 > @@ -0,0 +1,142 @@ > +/* > + * Tests for the arcname.c functions: > + * - ArcPathNormalize(), > + * - ArcPathToNtPath(). > + * > + * You should certainly fix the included headers before being able > + * to compile this file (as I didn't bother to have it compilable > + * under our (P/N)DK, but just under VS). > + */ Wine's test framework works okay for unit tests. You can make this an apitest by just #include'ing the C source file, then it can run on Testbot.

7 years, 6 months

Re: [ros-dev] [ros-diffs] [hbelusca] 74619: [USETUP]: filesup module: - Add a NtPathToDiskPartComponents helper, that takes in input a fully qualified NT path to a file on hard disk, e.g.: \Device\Harddisk1\Partition2\foo\b...

by Thomas Faber

On 2017-05-22 01:45, hbelusca(a)svn.reactos.org wrote: > +BOOLEAN > +NtPathToDiskPartComponents( > + IN PCWSTR NtPath, > + OUT PULONG pDiskNumber, > + OUT PULONG pPartNumber, > + OUT PCWSTR* PathComponent OPTIONAL) > +{ > + ULONG DiskNumber, PartNumber; > + PCWSTR Path; > + > + *pDiskNumber = 0; > + *pPartNumber = 0; > + if (PathComponent) *PathComponent = NULL; > + > + Path = NtPath; > + > + if (_wcsnicmp(Path, L"\\Device\\Harddisk", 16) != 0) > + { > + /* The NT path doesn't start with the prefix string, thus it cannot be a hard disk device path */ > + DPRINT1("'%S' : Not a possible hard disk device.\n", NtPath); > + return FALSE; > + } > + > + Path += 16; Avoiding these magic numbers would be nice. Options include: * static string à la static const WCHAR DeviceHarddiskPath[] = L"\\Device\\Harddisk"; _wcsnicmp(Path, DeviceHarddiskPath, ARRAYSIZE(DeviceHarddiskPath) - 1) * simply using wcslen(L"\\Device\\Harddisk") (which should get optimized) * making a "starts with" function * using RtlPrefixUnicodeString > @@ -593,6 +705,31 @@ > { > DPRINT1("Failed to open file %wZ, Status 0x%08lx\n", &Name, Status); > return Status; > + } > + > + if (FileSize) > + { > + /* Query the file size */ > + FILE_STANDARD_INFORMATION FileInfo; > + Status = NtQueryInformationFile(*FileHandle, > + &IoStatusBlock, > + &FileInfo, > + sizeof(FileInfo), > + FileStandardInformation); > + if (!NT_SUCCESS(Status)) > + { > + DPRINT("NtQueryInformationFile() failed (Status %lx)\n", Status); > + NtClose(*FileHandle); > + *FileHandle = NULL; > + return Status; > + } > + > + if (FileInfo.EndOfFile.HighPart != 0) > + DPRINT1("WARNING!! The file %wZ is too large!\n", Name); Did you mean &Name?

7 years, 6 months

Re: [ros-dev] [ros-diffs] [hbelusca] 74618: [USETUP]: Move all the code that retrieves the version resources from PE executable, out of osdetect.c, and place it in its own module inside the setuplib. Remove also some commen...

by Thomas Faber

On 2017-05-22 01:41, hbelusca(a)svn.reactos.org wrote: > +static PCVS_VERSION_INFO_STRUCT32 > +VersionInfo32_FindChild( > + IN PCVS_VERSION_INFO_STRUCT32 info, > + IN PCWSTR szKey, > + IN UINT cbKey) > +{ > + PCVS_VERSION_INFO_STRUCT32 child = VersionInfo32_Children(info); > + > + while ((ULONG_PTR)child < (ULONG_PTR)info + info->wLength) > + { > + if (!_wcsnicmp(child->szKey, szKey, cbKey) && !child->szKey[cbKey]) You're using cbKey in two places that need a character count here. > + return child; > + > + if (child->wLength == 0) return NULL; > + child = VersionInfo32_Next(child); > + } > + > + return NULL; > +}

7 years, 6 months

Re: [ros-dev] [ros-diffs] [hbelusca] 74617: [USETUP]: PartList module: Add a couple of disk/partition getters: GetDiskByBiosNumber, GetDiskByNumber, GetDiskBySCSI, GetDiskBySignature, GetPartition, GetDiskOrPartition. They ...

by Thomas Faber

On 2017-05-22 01:36, hbelusca(a)svn.reactos.org wrote: > --- branches/setup_improvements/base/setup/lib/partlist.c [iso-8859-1] (original) > +++ branches/setup_improvements/base/setup/lib/partlist.c [iso-8859-1] Sun May 21 23:36:13 2017 > @@ -1333,8 +1348,204 @@ > RtlFreeHeap(ProcessHeap, 0, List); > } > > +PDISKENTRY > +GetDiskByBiosNumber( > + IN PPARTLIST List, > + IN ULONG BiosDiskNumber) > +{ > + PDISKENTRY DiskEntry; > + PLIST_ENTRY Entry; > + > + /* Check for empty disks */ > + if (IsListEmpty(&List->DiskListHead)) > + return NULL; These empty list checks look superfluous.

7 years, 6 months

A surprising epic win: Wolfram Mathematica on ReactOS

by Hermès BÉLUSCA-MAÏTO

Hi all, I was pleasantly surprised to notice that the symbolic computation program « Wolfram Mathematica », in its version 8.0.4 (later versions are for Vista+ only), actually works quite pretty well on ReactOS (as of r74550): see the screen-capture at http://i.imgur.com/KegyW79.png . This software is broadly used in many scientific, engineering, mathematical, and computing fields. This is for example used intensively in theoretical physics (in high-energy physics, statistical physics etc.) due to the existence of numerous packages that have been developed for it. The only problem of this software resides in its price On the screen-capture I demonstrate an infinitesimal portion of what Mathematica can do: for the non-connoisseurs :) the picture shows the computation of the integral (area under the curve) of the 1-dimensional Gaussian function over the whole real axis (the result is square-root of pi), and then I plot in 3 dimensions the map of a 2-D Gaussian function, where on the horizontal plane there are the x and y axes, and the vertical axis is the z axis. The 3-D graph can be rotated, zoomed-in and out with the mouse, it can be edited, etc The software also proposes an extensive and interactive help documentation. Finally I show the fancy text-mode only interface of Mathematica, that nobody actually uses (it is only used when computing stuff in batch-mode). There are almost no graphical glitches. Few font problems can be seen (see the about-box, and the minus signs on the graph replaced by boxes). There are few keyboard shortcut problems such as pressing (on French keyboards) AltGr-5 to enter [ doesnt work, but anyways Enjoy! Best, Hermès

7 years, 6 months

Re: [ros-dev] About the recent USETUP improvements in the branch

by Robert Naumann

I'd say it should stay as fallback solution. Am 21.05.2017 18:27 schrieb "Colin Finck" <colin(a)reactos.org>: Hey Hermès, Great work on the setup components! I especially like the library that paves the way for a feature-rich graphical installer :) Am 18.05.2017 um 20:02 schrieb Hermès BÉLUSCA-MAÏTO: > P.S.: Using a 8x8 font for the installer quickly hurts the eyes (and it > has been like that since ages). It would be great to use a more readable > resolution / font size… Oh yes, I'm all for a good looking installer with nicely readable texts in all languages. Which brings us to the GUI installer and to the question what we would still need the text-mode installer for when the GUI one is perfect? - Colin _______________________________________________ Ros-dev mailing list Ros-dev(a)reactos.org http://www.reactos.org/mailman/listinfo/ros-dev

7 years, 6 months

About the recent USETUP improvements in the branch

by Hermès BÉLUSCA-MAÏTO

Hi all, Im writing a short mail to explain the recent USETUP improvements Im doing in the https://svn.reactos.org/svn/reactos/branches/setup_improvements/base/setup/ branch. The aim is three-fold: - To improve multi-OSes / bootloaders detection and solve https://jira.reactos.org/browse/CORE-4870 , https://jira.reactos.org/browse/CORE-12672 , https://jira.reactos.org/browse/CORE-13188 , https://jira.reactos.org/browse/CORE-13226 in a consistent & reliable way, and possibly having a look at https://jira.reactos.org/browse/CORE-13205 and https://jira.reactos.org/browse/CORE-10898 (low priority for me at the moment). - A bit related to the previous point, have the possibility to automatically detect existing installations of ReactOS (and of Windows) to allow simple upgrade. For this latter part I have code that almost works: it browses all the available partitions on all the disks, and for each, attempts to see whether either freeldr or ntldr is installed, and if so, open their respective configuration files, browse the latter, and from that, deduce the candidate NT installations. On the following screenshot : http://i.imgur.com/Nq67J1o.png , a Windows installation actually exists on partition(1) but is not listed because it happens to be on an NTFS partition. Then there is a ReactOS installation on a FAT partition(2), and finally a Windows installation on another FAT partition(4) (all on the same disk). The installation detector has respectively parsed freeldr.ini, and boot.ini, to deduce the installations, and have removed installations having the same disk and partition numbers and SystemRoot (otherwise you would have seen many ReactOS (xxx) entries). - To facilitate the future implementation of the 1st-stage GUI setup by someone, all the code that can be made common between USETUP and this future GUI setup, is placed into a library. At the time of writing, this concerns the so-called generic lists functionality as well as the partition lists & some file-system-related routines. It will certainly grow up later. Cheers, Hermès P.S.: Using a 8x8 font for the installer quickly hurts the eyes (and it has been like that since ages). It would be great to use a more readable resolution / font size

7 years, 6 months

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

2004

Ros-dev May 2017