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.