Here's a new patch that fixes all the issues mentioned before. Note that this isn't final, I didn't want to replace all calls to ::stat so that it's easier to review.<div><br></div><div>I have only one more questions:</div>
<div><br></div><div>1. _wopen accepts _stat64i32 instead of stat structure. These two are exactly the same, the only difference is that stat uses time_t and _stat64i32 uses __time64_t (time_t is a typedef for __time64_t but this depends on one more macro). I check to see that size of these two structures is the same, but is there something more that I could do? I'd also like to use some kind of static assert instead of runtime assert.</div>
<div><br></div><div>I've also noticed that function LowercaseString (the same applies to UppercaseString) doesn't work with UTF8 input. It's calling isupper and tolower and these functions can't handle UTF8. This should get a new issue probably?<br>
<br><div class="gmail_quote">On Wed, Sep 21, 2011 at 1:41 AM, Aaron Ballman <span dir="ltr"><<a href="mailto:aaron@aaronballman.com">aaron@aaronballman.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div class="im">On Tue, Sep 20, 2011 at 4:15 PM, Nikola Smiljanic <<a href="mailto:popizdeh@gmail.com">popizdeh@gmail.com</a>> wrote:<br>
> OK since this approach makes sense I'll shoot with my questions :)<br>
> 1. Where should get_utf8_argv go and is the name of this function OK? Right<br>
> now the function is inside llvm::sys::fs namespace because I need access to<br>
> Windows.h, should I leave it there.<br>
<br>
</div>I don't think it belongs there as it has nothing to do with the file<br>
system. However, I'm not familiar enough to know of a better place<br>
for it either. Hopefully someone else can chime in with an idea.<br>
<div class="im"><br>
> 2. get_utf8_argv allocates new char** representing utf8 argv, should I<br>
> deallocate this inside driver.cpp, use std::vector<std::string> instead or<br>
> ignore the allocation completely?<br>
<br>
</div>Perhaps some SmallVectors would make sense here? An inner one to hold<br>
the arg data, and an outer one to hold the list of inner vectors.<br>
Then you can do the cleanup within the call.<br>
<div class="im"><br>
> 3. There is an #ifdef inside driver.cpp right now that I don't like.<br>
> get_utf8_argv works for windows only, but I could change the function to<br>
> accept argv and return it as vector of utf8 strings. It would convert char*<br>
> to std::string on unix and use GetCommandLine + utf8 conversion on windows.<br>
<br>
</div>I have no concrete opinion one way or the other on this.<br>
<div class="im"><br>
> 4. Should I wrap functions like ::fstat and ::close for consistency even<br>
> though they don't work with paths?<br>
<br>
</div>I don't believe so.<br>
<div class="im"><br>
> I'll fix everything that is formatting related. You're right, current patch<br>
> has Open only for windows but I'll add the other one as well.<br>
<br>
</div>Thanks for the help!<br>
<font color="#888888"><br>
~Aaron<br>
</font></blockquote></div><br></div>