-
Notifications
You must be signed in to change notification settings - Fork 27
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
Fix issue #46 #48
Fix issue #46 #48
Conversation
Added new Annotation (Unsigned) used to indicate that an integer type or integer type pointer refers to an unsigned value. Used Unsigned-Annotation to mark parameter length in getDokanControlList method in DokanControl and all calls that lead to it Changed getDokanControlList to use an unsigned int instead of a long (matching Dokany that uses ULONG) and finished method (removed TODO) Changed length of array MountPoint to 260 to match Dokany
Updated DokanControl to use WinNT.MAX_PATH and rewrote getDokanControlList
Corrected types of fields in structures to match Dokany FIxed wrong types/usage in Constructor in DokanControl FIxed wrong parameters in Constructor in DokanOptions Fixed wrong parameters and added Unsigned-Annotation in accessors of the constructer
Added Unsigned-Annotation to all accessors of unsigned value Added UnsignedNumbers class with conversion methods between unsigned integer types and String Fixed toString method of DokanFileInfo and DokanOptions
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.
Generally I approve, but the (already previously) discussed changes need to be implemented.
Moved my comment and took out the sass to make it more professional See: dokan-dev#48 (comment)
Replaced usage of "assert" with explicit check to make sure that the check is done, even if assertions are disabled. From the java language guide (linked below): "Do not use assertions for argument checking in public methods. Argument checking is typically part of the published specifications (or contract) of a method, and these specifications must be obeyed whether assertions are enabled or disabled." The next sentence "Another problem with using assertions for argument checking is that erroneous arguments should result in an appropriate runtime exception (such as IllegalArgumentException, IndexOutOfBoundsException, or NullPointerException). An assertion failure will not throw an appropriate exception." doesn't apply in this case as this method isn't part of the API. It's only used internally and this check is implemented solely for the purpose of making sure everything in this library is working fine. See: https://docs.oracle.com/javase/7/docs/technotes/guides/language/assert.html or https://web.archive.org/web/20200622193110/https://docs.oracle.com/javase/7/docs/technotes/guides/language/assert.html
To document the reasoning for creating MAPPING.md:
|
Added methods to compare, divide and get the remainder of unsigned numbers to UnsignedNumbers. See: dokan-dev#48 (comment)
Added methods for unsigned conversion between integer types
Added constants that were missing from my last commit to UnsignedNumbers to allow compilation/fix compiler errors. Sorry!
Removed all unnecessary values from the @ Targets of @ Unsigned
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.
Add the Javadoc for the annotation and this PR is ready to be merged.
* At least the following cases ("minimum usage") should be annotated with <i>@Unsigned</i> to | ||
* guarantee the best quality of code: |
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.
In the future it may be useful to check our/foreign source code for compliance to this rule, e.g. with the Checker Framework.
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.
// Do not resolve or close. //
Added MAPPING.md as common guideline for type-conversions
Added new Annotation (Unsigned) used to indicate that an integer type or integer type pointer refers to an unsigned value.
Corrected types of fields in structures to match Dokany
Added Unsigned-Annotation to all accessors of unsigned values
Added UnsignedNumbers class with conversion methods between unsigned integer types and String
Merging this PR closes issue #46