Re: msi: Add an initial implementation of the PathEdit control

[Mike McCormack]

James Hawkins wrote:
> @@ -65,6 +65,7 @@ struct msi_control_tag
>      float progress_current;
>      float progress_max;
>      WCHAR name[1];
> +    DWORD attributes;

You shouldn't add members after name[1], as the string in name[] will 
overwrite them.
> +    sz = 0x20;
> +    buf = msi_alloc( sz*sizeof(WCHAR) );
> +    while( buf )
> +    {
> +        r = GetWindowTextW( control->hwnd, buf, sz );
> +        if( r < (sz-1) )
> +            break;
> +        sz *= 2;
> +        buf = msi_realloc( buf, sz*sizeof(WCHAR) );
> +    }

There's already code to get the window text in 
msi_dialog_edit_handler(), so please factor it out into a new function 
rather than copying it.  (maybe LPWSTR msi_get_window_text(HWND)?)
> +    LPCWSTR prop;
> +    prop = (LPWSTR)MSI_RecordGetString( rec, 9 );

You shouldn't need a cast here...

Mike

msi: Add an initial implementation of the PathEdit control

[James Hawkins]

On 8/23/06, Mike McCormack <mike(a)codeweavers.com> wrote:
> James Hawkins wrote:
> > @@ -65,6 +65,7 @@ struct msi_control_tag
> >      float progress_current;
> >      float progress_max;
> >      WCHAR name[1];
> > +    DWORD attributes;
>
> You shouldn't add members after name[1], as the string in name[] will
> overwrite them.

oops that's an obvious one.
> > +    sz = 0x20;
> > +    buf = msi_alloc( sz*sizeof(WCHAR) );
> > +    while( buf )
> > +    {
> > +        r = GetWindowTextW( control->hwnd, buf, sz );
> > +        if( r < (sz-1) )
> > +            break;
> > +        sz *= 2;
> > +        buf = msi_realloc( buf, sz*sizeof(WCHAR) );
> > +    }
>
> There's already code to get the window text in
> msi_dialog_edit_handler(), so please factor it out into a new function
> rather than copying it.  (maybe LPWSTR msi_get_window_text(HWND)?)

Can do.
> > +    LPCWSTR prop;
> > +    prop = (LPWSTR)MSI_RecordGetString( rec, 9 );
>
> You shouldn't need a cast here...

Yea, I need to take that back out.  For a while I was using a non-const prop.

Thanks,
James Hawkins