[PATCH v3 1/2] mspatcha: Implement ApplyPatchToFile and related functions.
Hans Leidekker
hans at codeweavers.com
Tue May 7 05:59:46 CDT 2019
Hi Conor,
Thanks for your contribution. The patches look good overall but I do
have some comments, mostly involving Wine conventions.
> create mode 100755 dlls/mspatcha/lzxd_dec.c
> create mode 100755 dlls/mspatcha/lzxd_dec.h
> create mode 100755 dlls/mspatcha/pa19.c
> create mode 100755 dlls/mspatcha/pa19.h
I assume that Alexandre's scripts will fix this up, but notice that
these files have the executable bit set.
>diff --git a/dlls/mspatcha/mspatcha.spec b/dlls/mspatcha/mspatcha.spec
> index cc8ca94..8c014b9 100644
> --- a/dlls/mspatcha/mspatcha.spec
> +++ b/dlls/mspatcha/mspatcha.spec
> @@ -1,12 +1,16 @@
> -1 stdcall ApplyPatchToFileA(str str str long)
> -2 stub ApplyPatchToFileByHandles
> -3 stub ApplyPatchToFileByHandlesEx
> -4 stub ApplyPatchToFileExA
> -5 stub ApplyPatchToFileExW
> -6 stdcall ApplyPatchToFileW(wstr wstr wstr long)
> -7 stdcall GetFilePatchSignatureA(str long ptr long ptr long ptr long ptr)
> -8 stub GetFilePatchSignatureByHandle
> -9 stdcall GetFilePatchSignatureW(wstr long ptr long ptr long ptr long ptr)
> -10 stub TestApplyPatchToFileA
> -11 stub TestApplyPatchToFileByHandles
> -12 stub TestApplyPatchToFileW
> +@ stdcall ApplyPatchToFileA(str str str long)
> +@ stdcall ApplyPatchToFileByBuffers(ptr long ptr long ptr long ptr ptr long ptr ptr)
> +@ stdcall ApplyPatchToFileByHandles(ptr ptr ptr long)
> +@ stdcall ApplyPatchToFileByHandlesEx(ptr ptr ptr long ptr ptr)
> +@ stdcall ApplyPatchToFileExA(str str str long ptr ptr)
> +@ stdcall ApplyPatchToFileExW(wstr wstr wstr long ptr ptr)
> +@ stdcall ApplyPatchToFileW(wstr wstr wstr long)
> +@ stdcall GetFilePatchSignatureA(str long ptr long ptr long ptr long str)
> +@ stdcall GetFilePatchSignatureByBuffer(ptr long long ptr long ptr long ptr long str)
> +@ stdcall GetFilePatchSignatureByHandle(ptr long ptr long ptr long ptr long str)
> +@ stdcall GetFilePatchSignatureW(wstr long ptr long ptr long ptr long wstr)
The last parameter to the signature functions is an output buffer,
which should be marked as 'ptr'.
> +@ stdcall NormalizeFileForPatchSignature(ptr long long ptr long long long ptr long ptr)
> +@ stdcall TestApplyPatchToFileA(str str long)
> +@ stdcall TestApplyPatchToFileByBuffers(ptr long ptr long ptr long)
> +@ stdcall TestApplyPatchToFileByHandles(ptr ptr long)
> +@ stdcall TestApplyPatchToFileW(wstr wstr long)
> diff --git a/dlls/mspatcha/mspatcha_main.c b/dlls/mspatcha/mspatcha_main.c
> index f78b8ab..3a311bc 100644
> --- a/dlls/mspatcha/mspatcha_main.c
> +++ b/dlls/mspatcha/mspatcha_main.c
> @@ -2,6 +2,7 @@
> * PatchAPI
> *
> * Copyright 2011 David Hedberg for CodeWeavers
> + * Copyright 2019 Conor McCarthy (implementations)
> *
> * This library is free software; you can redistribute it and/or
> * modify it under the terms of the GNU Lesser General Public
> @@ -16,6 +17,31 @@
> * You should have received a copy of the GNU Lesser General Public
> * License along with this library; if not, write to the Free Software
> * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
> + *
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library 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
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
Duplicate copyright text.
...
> /*****************************************************
> - * ApplyPatchToFileA (MSPATCHA.1)
> + * TestApplyPatchToFileA (MSPATCHA.@)
> */
> -BOOL WINAPI ApplyPatchToFileA(LPCSTR patch_file, LPCSTR old_file, LPCSTR new_file, ULONG apply_flags)
> +BOOL WINAPI TestApplyPatchToFileA(LPCSTR patch_file, LPCSTR old_file, ULONG apply_flags)
> {
> BOOL ret;
> - WCHAR *patch_fileW, *new_fileW, *old_fileW = NULL;
> + WCHAR *patch_fileW, *old_fileW = NULL;
>
> - if (!(patch_fileW = strdupAW( patch_file ))) return FALSE;
> - if (old_file && !(old_fileW = strdupAW( old_file )))
> + if (!(patch_fileW = strdupAW(patch_file))) return FALSE;
> + if (old_file && !(old_fileW = strdupAW(old_file)))
> {
> - HeapFree( GetProcessHeap(), 0, patch_fileW );
> + HeapFree(GetProcessHeap(), 0, patch_fileW);
> return FALSE;
> }
> - if (!(new_fileW = strdupAW( new_file )))
> + ret = apply_patch_to_file(patch_fileW, old_fileW, NULL, apply_flags, NULL, NULL, TRUE);
> + HeapFree(GetProcessHeap(), 0, patch_fileW);
> + if(old_fileW)
> + HeapFree(GetProcessHeap(), 0, old_fileW);
There's no need to check for NULL, HeapFree will do that for you.
> +BOOL WINAPI TestApplyPatchToFileByBuffers(BYTE *patch_file_buf, ULONG patch_file_size,
> + BYTE *old_file_buf, ULONG old_file_size,
> + ULONG* new_file_size,
> + ULONG apply_option_flags)
> +{
> + /* preserve last error on success as per windows */
> + DWORD last = GetLastError();
> +
> + DWORD err = apply_patch_to_file_by_buffers(patch_file_buf, patch_file_size,
> + old_file_buf, old_file_size,
> + NULL, 0, new_file_size, NULL,
> + apply_option_flags,
> + NULL, NULL,
> + TRUE);
> +
> + SetLastError(err == ERROR_SUCCESS ? last : err);
You can ignore last error on success until we find an application that
depends on it, which is unlikely.
> +/*****************************************************
> + * ApplyPatchToFileExA (MSPATCHA.@)
> + */
> +BOOL WINAPI ApplyPatchToFileExA(LPCSTR patch_file, LPCSTR old_file, LPCSTR new_file, ULONG apply_flags,
> + PPATCH_PROGRESS_CALLBACK progress_fn, PVOID progress_ctx)
> +{
> + BOOL ret = FALSE;
> + WCHAR *patch_fileW, *new_fileW, *old_fileW = NULL;
> +
> + if (!(patch_fileW = strdupAW(patch_file))) return FALSE;
> + if (old_file && !(old_fileW = strdupAW(old_file)))
> {
> - HeapFree( GetProcessHeap(), 0, patch_fileW );
> - HeapFree( GetProcessHeap(), 0, old_fileW );
> + HeapFree(GetProcessHeap(), 0, patch_fileW);
> return FALSE;
> }
> - ret = ApplyPatchToFileW( patch_fileW, old_fileW, new_fileW, apply_flags );
> - HeapFree( GetProcessHeap(), 0, patch_fileW );
> - HeapFree( GetProcessHeap(), 0, old_fileW );
> - HeapFree( GetProcessHeap(), 0, new_fileW );
> + if (!(new_fileW = strdupAW(new_file)))
> + {
> + goto free_wstrs;
> + }
> + ret = apply_patch_to_file(patch_fileW, old_fileW, new_fileW, apply_flags, progress_fn, progress_ctx, FALSE);
> + HeapFree(GetProcessHeap(), 0, new_fileW);
> +free_wstrs:
> + HeapFree(GetProcessHeap(), 0, patch_fileW);
> + if(old_fileW)
> + HeapFree(GetProcessHeap(), 0, old_fileW);
Redundant NULL check.
...
> +/*****************************************************
> + * GetFilePatchSignatureA (MSPATCHA.@)
> */
> BOOL WINAPI GetFilePatchSignatureA(LPCSTR filename, ULONG flags, PVOID data, ULONG ignore_range_count,
> - PPATCH_IGNORE_RANGE ignore_range, ULONG retain_range_count,
> - PPATCH_RETAIN_RANGE retain_range, ULONG bufsize, LPSTR buffer)
> + PPATCH_IGNORE_RANGE ignore_range, ULONG retain_range_count,
> + PPATCH_RETAIN_RANGE retain_range, ULONG bufsize, LPSTR buffer)
> {
> FIXME("stub - %s, %x, %p, %u, %p, %u, %p, %u, %p\n", debugstr_a(filename), flags, data,
> - ignore_range_count, ignore_range, retain_range_count, retain_range, bufsize, buffer);
> + ignore_range_count, ignore_range, retain_range_count, retain_range, bufsize, buffer);
> SetLastError(ERROR_CALL_NOT_IMPLEMENTED);
> return FALSE;
> }
>
> /*****************************************************
> - * GetFilePatchSignatureW (MSPATCHA.9)
> + * GetFilePatchSignatureW (MSPATCHA.@)
> */
> BOOL WINAPI GetFilePatchSignatureW(LPCWSTR filename, ULONG flags, PVOID data, ULONG ignore_range_count,
> - PPATCH_IGNORE_RANGE ignore_range, ULONG retain_range_count,
> - PPATCH_RETAIN_RANGE retain_range, ULONG bufsize, LPWSTR buffer)
> + PPATCH_IGNORE_RANGE ignore_range, ULONG retain_range_count,
> + PPATCH_RETAIN_RANGE retain_range, ULONG bufsize, LPWSTR buffer)
> {
> FIXME("stub - %s, %x, %p, %u, %p, %u, %p, %u, %p\n", debugstr_w(filename), flags, data,
> - ignore_range_count, ignore_range, retain_range_count, retain_range, bufsize, buffer);
> + ignore_range_count, ignore_range, retain_range_count, retain_range, bufsize, buffer);
> + SetLastError(ERROR_CALL_NOT_IMPLEMENTED);
> + return FALSE;
> +}
> +
> +/*****************************************************
> + * GetFilePatchSignatureByHandle (MSPATCHA.@)
> + */
> +BOOL WINAPI GetFilePatchSignatureByHandle(HANDLE handle, ULONG flags, PVOID options, ULONG ignore_range_count,
> + PPATCH_IGNORE_RANGE ignore_range, ULONG retain_range_count,
> + PPATCH_RETAIN_RANGE retain_range, ULONG bufsize, LPSTR buffer)
> +{
> + FIXME("stub - %p, %x, %p, %u, %p, %u, %p, %u, %p\n", handle, flags, options,
> + ignore_range_count, ignore_range, retain_range_count, retain_range, bufsize, buffer);
> + SetLastError(ERROR_CALL_NOT_IMPLEMENTED);
> + return FALSE;
> +}
> +
> +/*****************************************************
> + * GetFilePatchSignatureByBuffer (MSPATCHA.@)
> + */
> +BOOL WINAPI GetFilePatchSignatureByBuffer(PBYTE file_buf, ULONG file_size, ULONG flags, PVOID options,
> + ULONG ignore_range_count, PPATCH_IGNORE_RANGE ignore_range,
> + ULONG retain_range_count, PPATCH_RETAIN_RANGE retain_range,
> + ULONG bufsize, LPSTR buffer)
> +{
> + FIXME("stub - %p, %u, %x, %p, %u, %p, %u, %p, %u, %p\n", file_buf, file_size, flags, options,
> + ignore_range_count, ignore_range, retain_range_count, retain_range, bufsize, buffer);
> SetLastError(ERROR_CALL_NOT_IMPLEMENTED);
> return FALSE;
> }
> +
> +/*****************************************************
> + * NormalizeFileForPatchSignature (MSPATCHA.@)
> + */
> +INT WINAPI NormalizeFileForPatchSignature(PVOID file_buffer, ULONG file_size, ULONG flags, PATCH_OPTION_DATA *options,
> + ULONG new_coff_base, ULONG new_coff_time, ULONG ignore_range_count, PPATCH_IGNORE_RANGE ignore_range,
> + ULONG retain_range_count, PPATCH_RETAIN_RANGE retain_range)
> +{
> + FIXME("stub - %p, %u, %x, %p, %u, %u, %u, %p, %u, %p\n", file_buffer, file_size, flags, options, new_coff_base,
> + new_coff_time, ignore_range_count, ignore_range, retain_range_count, retain_range);
> + SetLastError(ERROR_CALL_NOT_IMPLEMENTED);
> + return 0;
> +}
Please split this patch where possible to reduce its size. You could
send the patchapi.h changes first as a separate patch and these stubs
could also go into a separate patch.
More information about the wine-devel
mailing list