Porting Wevtapi.dll(retry)#723
Conversation
Porting Winevt.h Adding Wevtutil
matthiasblaesing
left a comment
matthiasblaesing
left a comment
There was a problem hiding this comment.
Don't be alarmed, there are a few left over pieces, but overall it looks good.
| * 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. | ||
| */ |
There was a problem hiding this comment.
Please adjust the license header. The project is dua-licensed as LGPL 2.1 or later and AL 2.0. The source files were missed when the relicensing happend, which will be corrected by:
The intended header would be:
/* Copyright (c) 2016 Minoru Sakamoto, All Rights Reserved
*
* The contents of this file is dual-licensed under 2
* alternative Open Source/Free licenses: LGPL 2.1 or later and
* Apache License 2.0. (starting with JNA version 4.0.0).
*
* You can freely decide which license you want to apply to
* the project.
*
* You may obtain a copy of the LGPL License at:
*
* http://www.gnu.org/licenses/licenses.html
*
* A copy is also included in the downloadable source code package
* containing JNA, in file "LGPL2.1".
*
* You may obtain a copy of the Apache License at:
*
* http://www.apache.org/licenses/
*
* A copy is also included in the downloadable source code package
* containing JNA, in file "AL2.0".
*/
There was a problem hiding this comment.
It might be better to to change Contributing.md.
There was a problem hiding this comment.
Aggreed, that why that change is already part of the PR. The PR is unmerged as of now as I'm soliciting feedback on this massive change.
| * the required buffer size if the function fails with ERROR_INSUFFICIENT_BUFFER. | ||
| * @return The return value is ERROR_SUCCESS if the call succeeded; otherwise, a Win32 error code. | ||
| */ | ||
| int EvtGetExtendedStatus(int BufferSize, Pointer Buffer, IntByReference BufferUsed); |
There was a problem hiding this comment.
It should be save to use a char[] as the buffer, as this better aligns with the usage and is the default mapping. If a char[] is used, this works as expected:
char[] buffer = new char[1024];
EvtGetExtendedStatus(buffer.length, buffer, null);
String result = Native.toString(buffer):Expected in this case means: with bare memory you'd have to consider the width/size of wchar so multiply the value of BufferUsed by two to allocate the right amount of memory.
| * the {@link Kernel32#GetLastError} function. | ||
| */ | ||
| boolean EvtFormatMessage(EVT_HANDLE PublisherMetadata, EVT_HANDLE Event, int MessageId, int ValueCount, EVT_VARIANT[] Values, | ||
| int Flags, int BufferSize, Pointer Buffer, IntByReference BufferUsed); |
There was a problem hiding this comment.
Please see comment for line 90
| * call the {@link Kernel32#GetLastError} function. | ||
| */ | ||
| boolean EvtNextChannelPath(EVT_HANDLE ChannelEnum, int ChannelPathBufferSize, Pointer ChannelPathBuffer, | ||
| IntByReference ChannelPathBufferUsed); |
There was a problem hiding this comment.
Please see comment for line 90
| * otherwise, NULL. If NULL, call {@link Kernel32#GetLastError} function to get the error code. | ||
| */ | ||
| boolean EvtNextPublisherId(EVT_HANDLE PublisherEnum, int PublisherIdBufferSize, Pointer PublisherIdBuffer, | ||
| IntByReference PublisherIdBufferUsed); |
There was a problem hiding this comment.
Please see comment for line 90
| super(peer, Structure.ALIGN_DEFAULT, W32APITypeMapper.UNICODE); | ||
| } | ||
|
|
||
| protected ByReference newByReference() { |
There was a problem hiding this comment.
Please check if the the methods newByReference, newByValue and newInstance are needed. I see no reason for their implementation.
| throw new Win32Exception(Kernel32.INSTANCE.GetLastError()); | ||
| } | ||
|
|
||
| Memory buff = new Memory(1024); |
There was a problem hiding this comment.
This needs to be adjusted if you switch to char[] in Wevtapi. See 245 for the reason.
| IntByReference buffUsed = new IntByReference(); | ||
| while (true) { | ||
| buff.clear(); | ||
| if (!Wevtapi.INSTANCE.EvtNextChannelPath(channelHandle, (int) buff.size(), buff, buffUsed)) { |
There was a problem hiding this comment.
This is the reason char[] is a good ide, buff.size() is twice to large in this case. buff.size() is size in bytes, but EvtNextChannelPath expects size in chars.
| throw new Win32Exception(Kernel32.INSTANCE.GetLastError()); | ||
| } | ||
|
|
||
| Memory buff; |
There was a problem hiding this comment.
With the suggested changes to WevtapiUtil you'll get a String back directly, so in that case this needs adjusting.
| Wevtapi.INSTANCE.EvtClose(handle); | ||
| } | ||
| } | ||
| System.out.println(result); |
There was a problem hiding this comment.
This looks like an artifact from debugging. I was surprised when I saw the output and could not directly place it. Please remove.
change buffer type to `char[]`: - EvtGetExtendedStatus - EvtFormatMessage - EvtNextChannelPath - EvtNextPublisherId change return value of utilities to String: - EvtFormatMessage - EvtNextPublisherId add EVT_VARIANT#ByValue constructor remove EVT_PRC_LOGIN#newByReference/newByValue/newIncetance
matthiasblaesing
left a comment
matthiasblaesing
left a comment
There was a problem hiding this comment.
Thank you - I'll merge this into master.
…#723) Motivation: We need to use the correct calculated length when obtaining the ocid buffer as otherwise it might fail when operating on a slice Modifications: - Use the calculated length Result: Don't fail when operating on slices
2 participants
Porting Wevtapi.dll(retry)
Porting Winevt.h
Adding Wevtutil