Skip to content

Conversation

ghost
Copy link

@ghost ghost commented Feb 10, 2013

http://d.puremagic.com/issues/show_bug.cgi?id=7819

  • Moved setTimes implementation closer to getTimes.
  • Added FILE_FLAG_BACKUP_SEMANTICS and FILE_ATTRIBUTE_DIRECTORY flags to enable setting folder times on Windows (See MSDN docs for these here).
  • Updated getTimes docs to reflect it works on folders.
  • Changed enforce to cenforce for Posix in setTimes since the function claimed it only threw FileException's.

Credits to Jay Norwood for the patch for setTimes fix.

@CyberShadow
Copy link
Member

Should we really be using FILE_FLAG_BACKUP_SEMANTICS in a library function for setting directory times? It seems like that flag was created for a very specific purpose (backup software).

Also it should be noted that FILE_FLAG_BACKUP_SEMANTICS requires SE_BACKUP_NAME and SE_RESTORE_NAME privileges, so I believe this won't work for unprivileged processes (the default execution level for programs on Windows Vista and newer).

@ghost
Copy link
Author

ghost commented Feb 10, 2013

Hmm I see. Is there any other way we can implement this? It doesn't work if there's only FILE_ATTRIBUTE_DIRECTORY | FILE_ATTRIBUTE_NORMAL.

@CyberShadow
Copy link
Member

The logical conclusion is that the Windows API was designed such that setting directory times directly should not be possible with usual flags.

@ghost
Copy link
Author

ghost commented Feb 10, 2013

Heh, "designed" :). I can't find a flag which could help, tried looking in here, and various other places (GENERIC_WRITE doesn't work either).

Tried to look on SO, only thing I've found is http://stackoverflow.com/a/279944 and it says FILE_FLAG_BACKUP_SEMANTICS has to be used. And a file/dir time modifier from codeproject also uses this flag.

@CyberShadow
Copy link
Member

And a file/dir time modifier from codeproject also uses this flag.

I don't think that has any weight - unless the project author argumented their decision using arguments not mentioned in this discussion, the use of the flag is as uninformed as we are.

A better idea would be to check what standard libraries of other language implementations do.

@ghost
Copy link
Author

ghost commented Feb 10, 2013

A better idea would be to check what standard libraries of other language implementations do.

I'll get to it later, for now closing this. Thanks for the review, we avoided making a mistake.

@ghost ghost closed this Feb 10, 2013
@ghost
Copy link
Author

ghost commented Feb 10, 2013

Eeeehh, at the very bottom of CreateFile:

To open a directory using CreateFile, specify the FILE_FLAG_BACKUP_SEMANTICS 
flag as part of dwFlagsAndAttributes. Appropriate security checks still apply when
this flag is used without SE_BACKUP_NAME and SE_RESTORE_NAME privileges.

PHP also uses it, and everywhere I look that flag is used.

@ghost ghost reopened this Feb 10, 2013
@alexrp
Copy link
Contributor

alexrp commented Feb 10, 2013

LGTM. @CyberShadow ?

@CyberShadow
Copy link
Member

I'm fine with it if other languages do it. At least we have something to point a finger at :)

alexrp added a commit that referenced this pull request Feb 10, 2013
Issue 7819 - setTimes should support folders on Windows
@alexrp alexrp merged commit ad80aef into dlang:master Feb 10, 2013
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.

3 participants