Skip to content
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

Deprecate EventLogRecord#getEventId in favor of #getInstanceId #1105

Merged
merged 3 commits into from
Jun 12, 2019

Conversation

dbwiddis
Copy link
Contributor

The terms "Event ID" and "event identifier" refer to different things, which leads to confusion and probably led to .NET deprecating the former term. JNA should do the same.

@dbwiddis dbwiddis force-pushed the eventId branch 2 times, most recently from 61e6661 to ee4c606 Compare June 11, 2019 04:00
@dbwiddis dbwiddis changed the title Deprecate c.s.j.p.win32.Advapi32Util#getEventId in favor of #getInstanceId Deprecate EventLogRecord#getEventId in favor of #getInstanceId Jun 11, 2019
@dblock
Copy link
Member

dblock commented Jun 11, 2019

We don't seem to have tests for these, but maybe we can make some assumptions about the values returned from these methods in https://github.com/java-native-access/jna/blob/cc1acdac02e4d0dda93ba01bbe3a3435b8933dab/contrib/platform/test/com/sun/jna/platform/win32/Advapi32UtilTest.java? Ideally I'd want tests to describe something similar to http://balazsberki.com/2017/02/reading-the-windows-event-log-event-id-problems-with-qualifiers/ in.

Merge on 🍏 either way.

@dbwiddis
Copy link
Contributor Author

dbwiddis commented Jun 11, 2019

I think I can throw something together with some well-known ID types, e.g, event log started (6005) will be an info type with Instance ID -2147477643 (technically the positive value).

Actually that brings up another question: in .NET the Instance ID is a 64-bit number. Should we make that a long? (To be clear, it's a 32-bit unsigned integer...)

@dbwiddis
Copy link
Contributor Author

So I just added a test to search for the "Event Log started" event in the system log. But I also notided in Advapi32UtilTest the testReportEvent() method actually creates an event entry in the log that we could look for. It currently uses the event ID of 0 but we could customize that for our tests... I'll rework that.

@dblock dblock merged commit dabdcd7 into java-native-access:master Jun 12, 2019
@dblock
Copy link
Member

dblock commented Jun 12, 2019

👍

@dbwiddis dbwiddis deleted the eventId branch September 21, 2019 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants