New subject: Wine cabinet.dll FDI Conformance Test Patch

3 Mar 2005


      On Wed, Mar 02, 2005 at 03:41:21PM -0800, Rizwan Kassim wrote:
Good stuff guys! Here are a few comments:
+#ifndef STANDALONE
+#include <wine/test.h>
+#define ok2 ok
+#else
+/* To build outside Wine tree, compile with cl -DSTANDALONE -D_X86_ tvfs.c cabinet_fdi.c FDI.lib
+  These are only called if standalone are used, defining ok and START_TEST, which would normally be declared in winetree */
+#include <assert.h>
+#include <stdio.h>
+#define START_TEST(name) main(int argc, char **argv)
+#define ok(condition, msg)    \
+    do { if(!(condition)) {  \
+        fprintf(stderr,"failed at %d, msg:" msg "\n",__LINE__); \
+    exit(1);    \
+    } } while(0)
+#define ok2(condition, msg, arg) \
+    do { if(!(condition)) {  \
+        fprintf(stderr,"failed at %d, msg:" msg "\n",__LINE__, arg); \
+    exit(1);    \
+    } } while(0)
+#define todo_wine
Please get rid of this. It is useless in the Wine tree. If you need it,
just put it in a private header file, and include it there.
+#ifndef DEBUG_ALLOC
+FNALLOC(final_alloc) {
+    return malloc(cb);
+}
+FNFREE(final_free) {
+    free(pv);
+    return;
+}
+#else
+FNALLOC(final_alloc) {
+    trace("   FNALLOC just called with %d\n",cb);
+    return malloc(cb);
+}
+FNFREE(final_free) {
+    trace("   FNFREE just called with %d\n",pv);
+    free(pv);
+    return;
+}
+#endif
No need for this either. It's a test after all, there's no big deal if you
have a memory leak... :)
+#ifdef VERBOSE
+    trace("   FNFDINOTIFY real just called with %d, %d \n",fdint,pfdin);
+#endif
Don't do it this way. Define trace such that you can use it unconditionally.
In general, try to avoid using the preprocessor in the code.
+    trace("   Cabinet Data : cbC %d cF %d cFi %d si %d iC %d fr %d hp %d hn %d\n",
+        cabinfo.cbCabinet,
+        cabinfo.cFolders ,
+        cabinfo.cFiles ,
+        cabinfo.setID,
+        cabinfo.iCabinet,
+        cabinfo.fReserve ,
+        cabinfo.hasprev ,
+        cabinfo.hasnext );
These should fit on 2-3 lines, no?
+    ok2 ( cabinfo.cbCabinet == TcbCabinet, "FDIIsCabinet,cabinfo %s data did not match! Failed!\n", cabname);
+    ok2 ( cabinfo.cFolders == TcFolders, "FDIIsCabinet,cabinfo %s data did not match! Failed!\n", cabname);
+    ok2 ( cabinfo.cFiles == TcFiles, "FDIIsCabinet,cabinfo %s data did not match! Failed!\n", cabname);
+    ok2 ( cabinfo.setID == TsetID, "FDIIsCabinet,cabinfo %s data did not match! Failed!\n", cabname);
+    ok2 ( cabinfo.iCabinet == TiCabinet, "FDIIsCabinet,cabinfo %s data did not match! Failed!\n", cabname);
+    ok2 ( cabinfo.fReserve == TfReserve, "FDIIsCabinet,cabinfo %s data did not match! Failed!\n", cabname);
+    ok2 ( cabinfo.hasprev == Thasprev, "FDIIsCabinet,cabinfo %s data did not match! Failed!\n", cabname);
+    ok2 ( cabinfo.hasnext == Thasnext, "FDIIsCabinet,cabinfo %s data did not match! Failed!\n", cabname);
Use ok() instead of ok2(). Also, it's probably better if the message contains the values of the
variables that don't match, so we know why it failed.
+#ifdef TVFS_DEBUG
+    trace("tvfs_create called with %s, %d, %d\n", fname, buf, len);
+#endif
Similar to the one above, do the test inside the body of trace(), not on every use.
+    switch(whither) {
+    case SEEK_SET: {
+    handles[h]->pos = whence;
+    break;
+    }
+    case SEEK_CUR: {
+    handles[h]->pos += whence;
+    break;
+    }
This one needs a bit of indentation, no?
+int tvfs_close(int h){
+    int inode = handles[h]->inode;
+#ifdef TVFS_DEBUG
+    trace("tvfs_close called with %d\n", h);
+#endif
+    if (!files[inode]){
+    return -1;
+    }
+    free(handles[h]);
+    /* Currently does NOT enabled open to reuse this handle */
+    return 0;
+ }
The formatting on this is funny. It needs indenting, spaces before '{',
and the opening braket for the function on a new line.
+main(){
Ditto.
-- 
Dimi.

Re: Wine cabinet.dll FDI Conformance Test Patch