dlls/msi/streams.c -- simplify and constify
Gerald Pfeifer
gerald at pfeifer.com
Sat Nov 3 17:10:14 CDT 2007
On Sat, 3 Nov 2007, James Hawkins wrote:
> This change is wrong. If you'd actually read what the code intended to
> do instead of just fixing warnings, you'd see that add_streams_to_table
> returns -1 on error.
That's what I am actually doing -- trying to read the code, including all
invocations of functions that I modify. And doing so I have encountered
(and fixed) more than one real bug. It does mean I need to dig into many
different corner of Wine that I have never seen before and, well, errare
humanum est, so I really appreciate review and feedback like yours. ;-)
> While the check for < 0 is not correct, removing the check entirely
> is wrong. The check should be if (sv->num_rows == -1) return
> ERROR_FUNCTION_FAILED.
Good observation, though comparing an unsigned type against -1 is not
exactly good practice. Delving more into msi/streams.c, I came up with
the patch below which tries to address this (and some other issues).
If Alexandre and you prefer, I can change the check and really make it
just read sv->num_rows == -1. I just wanted to point out there also is
an alternative way.
Thanks again for your feedback. It was very helpful, and I'll watch
out for cases like this.
Gerald
Index: streams.c
===================================================================
RCS file: /home/wine/wine/dlls/msi/streams.c,v
retrieving revision 1.7
diff -u -3 -p -r1.7 streams.c
--- streams.c 18 Oct 2007 13:00:57 -0000 1.7
+++ streams.c 3 Nov 2007 22:01:13 -0000
@@ -39,7 +39,7 @@ WINE_DEFAULT_DEBUG_CHANNEL(msidb);
typedef struct tabSTREAM
{
- int str_index;
+ UINT str_index;
LPWSTR name;
IStream *stream;
} STREAM;
@@ -54,7 +54,7 @@ typedef struct tagMSISTREAMSVIEW
UINT row_size;
} MSISTREAMSVIEW;
-static BOOL add_stream_to_table(MSISTREAMSVIEW *sv, STREAM *stream, int index)
+static BOOL add_stream_to_table(MSISTREAMSVIEW *sv, STREAM *stream, UINT index)
{
if (index >= sv->max_streams)
{
@@ -315,7 +315,7 @@ static UINT STREAMS_modify(struct tagMSI
static UINT STREAMS_delete(struct tagMSIVIEW *view)
{
MSISTREAMSVIEW *sv = (MSISTREAMSVIEW *)view;
- int i;
+ UINT i;
TRACE("(%p)\n", view);
@@ -381,7 +381,8 @@ static const MSIVIEWOPS streams_ops =
NULL,
};
-static UINT add_streams_to_table(MSISTREAMSVIEW *sv)
+static int add_streams_to_table(MSISTREAMSVIEW *sv)
+ /* Return -1 in case of error. */
{
IEnumSTATSTG *stgenum = NULL;
STATSTG stat;
@@ -433,6 +434,7 @@ static UINT add_streams_to_table(MSISTRE
UINT STREAMS_CreateView(MSIDATABASE *db, MSIVIEW **view)
{
MSISTREAMSVIEW *sv;
+ INT i;
TRACE("(%p, %p)\n", db, view);
@@ -442,10 +444,12 @@ UINT STREAMS_CreateView(MSIDATABASE *db,
sv->view.ops = &streams_ops;
sv->db = db;
- sv->num_rows = add_streams_to_table(sv);
- if (sv->num_rows < 0)
- return ERROR_FUNCTION_FAILED;
+ i = add_streams_to_table(sv);
+ if (i < 0)
+ return ERROR_FUNCTION_FAILED;
+ else
+ sv->num_rows = i;
*view = (MSIVIEW *)sv;
More information about the wine-devel
mailing list