Re: [PATCH 03/12] msvcp110: Add tr2_sys__Open_dir implementation.(resend)

[Piotr Caban]

Hi,

On 09/07/15 16:21, YongHao Hu wrote:
> +/* ?_Open_dir(a)sys@tr2(a)std@@YAPAXAAY0BAE(a)DPBDAAHAAW4file_type@123@@Z */
> +/* ?_Open_dir(a)sys@tr2(a)std@@YAPEAXAEAY0BAE(a)DPEBDAEAHAEAW4file_type@123@@Z */
> +void* __cdecl tr2_sys__Open_dir(char* target, char const* dest, int* err_code, enum file_type* type)
> +{
> +    HANDLE handle;
> +    WIN32_FIND_DATAA data;
> +    char temppath[MAX_PATH];
> +    TRACE("(%s %s %p %p)\n", debugstr_a(target), debugstr_a(dest), err_code, type);
> +    strcpy(temppath, dest);
> +    strcat(temppath, "\\*.*");
> Is native limiting the path to MAX_PATH characters? What happens if 
> longer path is passed?
> +    handle = FindFirstFileA(temppath, &data);
> +    if(handle == INVALID_HANDLE_VALUE) {
> +        if(GetLastError() == ERROR_PATH_NOT_FOUND)
> +            *err_code = ERROR_BAD_PATHNAME;
> +        else
> +            *err_code = GetLastError();
> This doesn't look right. Probably some other function is returning 
> different error on windows and on wine (or different function is used 
> internally). Anyway I don't think we need to be compatible here (at 
> least as long as there's no application that depends on it).
> +    while(!strcmp(data.cFileName, ".") || !strcmp(data.cFileName, "..")) {
> +        if(!FindNextFileA(handle, &data)) {
> +            *err_code = ERROR_SUCCESS;
> +            *type = status_unknown;
> +            return NULL;
> You're leaking handle here.

Thanks,
Piotr

[PATCH 03/12] msvcp110: Add tr2_sys__Open_dir implementation.(resend)

[YongHao Hu]

Hi,

On 15/9/28 下午7:51, Piotr Caban wrote:
> Hi,
>
> On 09/07/15 16:21, YongHao Hu wrote:
> > +/* ?_Open_dir(a)sys@tr2(a)std@@YAPAXAAY0BAE(a)DPBDAAHAAW4file_type@123@@Z */
> > +/* 
> > ?_Open_dir(a)sys@tr2(a)std@@YAPEAXAEAY0BAE(a)DPEBDAEAHAEAW4file_type@123@@Z */
> > +void* __cdecl tr2_sys__Open_dir(char* target, char const* dest, int* 
> > err_code, enum file_type* type)
> > +{
> > +    HANDLE handle;
> > +    WIN32_FIND_DATAA data;
> > +    char temppath[MAX_PATH];
> > +    TRACE("(%s %s %p %p)\n", debugstr_a(target), debugstr_a(dest), 
> > err_code, type);
> > +    strcpy(temppath, dest);
> > +    strcat(temppath, "\\*.*");
> > Is native limiting the path to MAX_PATH characters? What happens if 
> > longer path is passed?

As far as I know, we can not create a file or directory when their path 
longer than MAX_PATH on Windows, so it is impossible to pass a longer 
valid path to tr2_sys__Open_dir.
If we pass a longer invalid path, I think that it's err_code would be 
ERROR_PATH_NOT_FOUND( GetLastError() )

Thank you very much. :)

[PATCH 03/12] msvcp110: Add tr2_sys__Open_dir implementation.(resend)

[Piotr Caban]

On 09/29/15 18:03, YongHao Hu wrote:
> Hi,
>
> On 15/9/28 下午7:51, Piotr Caban wrote:
> > Hi,
> >
> > On 09/07/15 16:21, YongHao Hu wrote:
> > > +/* ?_Open_dir(a)sys@tr2(a)std@@YAPAXAAY0BAE(a)DPBDAAHAAW4file_type@123@@Z */
> > > +/*
> > > ?_Open_dir(a)sys@tr2(a)std@@YAPEAXAEAY0BAE(a)DPEBDAEAHAEAW4file_type@123@@Z */
> > > +void* __cdecl tr2_sys__Open_dir(char* target, char const* dest, int*
> > > err_code, enum file_type* type)
> > > +{
> > > +    HANDLE handle;
> > > +    WIN32_FIND_DATAA data;
> > > +    char temppath[MAX_PATH];
> > > +    TRACE("(%s %s %p %p)\n", debugstr_a(target), debugstr_a(dest),
> > > err_code, type);
> > > +    strcpy(temppath, dest);
> > > +    strcat(temppath, "\\*.*");
> > > Is native limiting the path to MAX_PATH characters? What happens if
> > > longer path is passed?
>
> As far as I know, we can not create a file or directory when their path
> longer than MAX_PATH on Windows, so it is impossible to pass a longer
> valid path to tr2_sys__Open_dir.
> If we pass a longer invalid path, I think that it's err_code would be
> ERROR_PATH_NOT_FOUND( GetLastError() )
> You can specify a longer path by prepending it with "\\?\" prefix. Even 
> if longer paths are not supported you should probably detect that case 
> instead of possibly crashing in strcpy call.

Thanks,
Piotr