Re: [MSI 1/4] add streams table

[Mike McCormack]

Hi,

Thanks for submitting the patch, and keeping the coding style consistent!

Andrey Turkin wrote:
> This patch adds virtual _Streams table to MSI because native MSI
> maintains such table
>
> ChangeLog:
> virtual _Streams table added
> +static UINT STREAMS_fetch_stream( struct tagMSIVIEW *view, UINT row, UINT col, IStream **stm )
> +{

....
> -
> - /*
> -
> * The column marked with the type stream data seems to have a single number
> -
> * which references the column containing the name of the stream data
> -
> *
> -
> * Fetch the column to reference first.
> -
> */
> - r = view->ops->fetch_int( view, row, col, &ival );

I'm not so keen on the way this is done.

You seem to have duplicated alot of code from TABLE_fetch_stream here.
> - if( r != ERROR_SUCCESS )
> - {
> -
>    ERR("1: %d\n", r);
> -
>    return r;
> - }
> -
> - ERR("fetched %d\n", ival);

And forgotten to remove your debug code.
> +MSIVIEWOPS streams_ops =
> +{
> - TABLE_fetch_int,
> - STREAMS_fetch_stream,
> - TABLE_set_int,
> - TABLE_insert_row,
> - STREAMS_execute,
> - TABLE_close,
> - TABLE_get_dimensions,
> - TABLE_get_column_info,
> - TABLE_modify,
> - TABLE_delete,
> - TABLE_find_matching_rows
>
> +};

The streams table is just another table.  How about generating an 
MSITABLE structure containing the data rather than treat it as a special 
case in here?

Mike