Re: Wine cabinet.dll FDI Conformance Test Patch
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.
Dimitrie O. Paun wrote:
On Wed, Mar 02, 2005 at 03:41:21PM -0800, Rizwan Kassim wrote: +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... :) I disagree, you can't know how big the test will become nor if it will run on a Win95 box with something like 8 MB of RAM. And they need to learn to program correctly, that's one of the reasons for their work, so they need to free the memory too ;)
bye michael -- Michael Stefaniuc Tel.: +49-711-96437-199 Sr. Network Engineer Fax.: +49-711-96437-111 Red Hat GmbH Email: mstefani(a)redhat.com Hauptstaetterstr. 58 http://www.redhat.de/ D-70178 Stuttgart
participants (2)
-
Dimitrie O. Paun -
Michael Stefaniuc