-
Notifications
You must be signed in to change notification settings - Fork 1.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Porting Wevtapi.dll(retry) #723
Porting Wevtapi.dll(retry) #723
Conversation
Porting Winevt.h Adding Wevtutil
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be better to to change Contributing.md.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please see comment for line 90
super(peer, Structure.ALIGN_DEFAULT, W32APITypeMapper.UNICODE); | ||
} | ||
|
||
protected ByReference newByReference() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
Porting Wevtapi.dll(retry)
Porting Winevt.h
Adding Wevtutil