-
Notifications
You must be signed in to change notification settings - Fork 68
Added utime and sysFsUtime #84
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
Conversation
miigotu
left a comment
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.
@bucanero please review.
|
@miigotu sure, I'll take a look on the PR and the syscalls. about Cygwin, I don't have a windows environment so I can't really check those changes. Is there any Windows-dev around? @crystalct , do you have more Cygwin changes to submit? if you have, perhaps it will be better to create a brach like |
|
Yes, please keep changes separated. 1 for adding the syscall, another for changes like cygwin/Mac support. You don't need to create a new pr to do it, you can force push to the branch you made this PR with. |
bucanero
left a comment
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.
Here's my review. For utime(), please check the changes I suggested.
About cygwin changes, I don't have Windows so I couldn't test, but since it's limited in #ifdef __CYGWIN__ tags, it won't break the build in other platforms.
|
@crystalct please don't get discouraged and give up lol, we appreciate your contributions! |
|
It was my first pull request :P |
|
Branch created for cygwin. |
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.
utime() looks good 👍
btw, good catch finding that sysLv2FsUtime() doesn't handle a NULL parameter as expected.
grazie @crystalct ! 😄
|
About cygwin.. i confirm 32bit version has the same problem. In addition i discover that gcc inside a cygwin installation doesn't have WIN32 defined,bacause it's POSIX. So if in sources code is necessary a #ifdef WIN32, with cygwin it's not true. |
|
@bucanero, access still old not working version. |
No description provided.