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@sys@tr2@std@@YAPAXAAY0BAE@DPBDAAHAAW4file_type@123@@Z */
> +/* ?_Open_dir@sys@tr2@std@@YAPEAXAEAY0BAE@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@sys@tr2@std@@YAPAXAAY0BAE@DPBDAAHAAW4file_type@123@@Z */
> > +/* 
> > ?_Open_dir@sys@tr2@std@@YAPEAXAEAY0BAE@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@sys@tr2@std@@YAPAXAAY0BAE@DPBDAAHAAW4file_type@123@@Z */
> > > +/*
> > > ?_Open_dir@sys@tr2@std@@YAPEAXAEAY0BAE@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