fix inode/device usage for win32, take #1

hi, This is a first attempt (draft) to fix the pecl bug #16860 (http://pecl.php.net/bugs/bug.php?id=16860). Here are the changes I made so far: - add apc_ino_t and apc_dev_t - native ino_t/dev_t on non windows platform - unsigned int64 on windows - use string for the inode and device key (cache info and iterator) - device uses now the volume serial number (part of the file information) - inode uses now the file id (part of the file information) - add zend_file_handle support to apc_cache_make_file_key to avoid (no need to open/stat a file already opened) - to do: have to add fstat on unix - getfileinfofromhandle (stat-like) is used on windows I have tested it using fcgi and apache 2.2 on windows and everything seems to work as it should. However I like to test it further and fix any side effects before to apply it. Thomas Peterson also tested it using apache and CLI with the test cases provided in the report. Comments and suggestions welcome :) Cheers, -- Pierre http://blog.thepimp.net | http://www.libgd.org

Index: apc_main.c =================================================================== --- apc_main.c (revision 289689) +++ apc_main.c (working copy) @@ -521,7 +521,7 @@ #endif /* try to create a cache key; if we fail, give up on caching */ - if (!apc_cache_make_file_key(&key, h->filename, PG(include_path), t TSRMLS_CC)) { + if (!apc_cache_make_file_key(&key, h->filename, PG(include_path), t, h TSRMLS_CC)) { return old_compile_file(h, type TSRMLS_CC); } Index: php_apc.c =================================================================== --- php_apc.c (revision 289689) +++ php_apc.c (working copy) @@ -462,8 +462,21 @@ if(p->type == APC_CACHE_ENTRY_FILE) { add_assoc_string(link, "filename", p->data.file.filename, 1); +#ifdef PHP_WIN32 + { + char buf[20]; + sprintf(buf, "%I64d", p->data.file.device); + add_assoc_string(link, "device", buf, 1); + + sprintf(buf, "%I64d", p->data.file.inode); + add_assoc_string(link, "inode", buf, 1); + } +#else add_assoc_long(link, "device", p->data.file.device); add_assoc_long(link, "inode", p->data.file.inode); +#endif + + add_assoc_string(link, "type", "file", 1); if(APCG(file_md5)) { make_digest(md5str, p->data.file.md5); @@ -500,8 +513,20 @@ if(p->type == APC_CACHE_ENTRY_FILE) { add_assoc_string(link, "filename", p->data.file.filename, 1); +#ifdef PHP_WIN32 + { + char buf[20]; + sprintf(buf, "%I64d", p->data.file.device); + add_assoc_string(link, "device", buf, 1); + + sprintf(buf, "%I64d", p->data.file.inode); + add_assoc_string(link, "inode", buf, 1); + } +#else add_assoc_long(link, "device", p->data.file.device); add_assoc_long(link, "inode", p->data.file.inode); +#endif + add_assoc_string(link, "type", "file", 1); if(APCG(file_md5)) { make_digest(md5str, p->data.file.md5); @@ -1255,7 +1280,7 @@ file_handle.free_filename = 0; file_handle.opened_path = NULL; - if (!apc_cache_make_file_key(&(keys[i]), file_handle.filename, PG(include_path), t TSRMLS_CC)) { + if (!apc_cache_make_file_key(&(keys[i]), file_handle.filename, PG(include_path), t, NULL TSRMLS_CC)) { add_assoc_long(return_value, Z_STRVAL_PP(hentry), -1); /* -1: compilation error */ apc_wprint("Error compiling %s in apc_compile_file.", file_handle.filename); break; Index: apc_iterator.c =================================================================== --- apc_iterator.c (revision 289689) +++ apc_iterator.c (working copy) @@ -46,7 +46,11 @@ if (slot->key.type == APC_CACHE_KEY_FILE) { /* keys should be unique and with stat=1 we could have multiple files with the same name, so use '<device> <inode>' instead */ +#ifdef PHP_WIN32 + item->key_len = spprintf(&item->key, 0, "%I64d %I64d", slot->key.data.file.device, slot->key.data.file.inode); +#else item->key_len = spprintf(&item->key, 0, "%ld %ld", (ulong)slot->key.data.file.device, (ulong)slot->key.data.file.inode); +#endif item->filename_key = estrdup(slot->value->data.file.filename); } else if (slot->key.type == APC_CACHE_KEY_USER) { item->key = estrndup((char*)slot->key.data.user.identifier, slot->key.data.user.identifier_len); @@ -80,12 +84,24 @@ } if (APC_ITER_DEVICE & iterator->format) { if(slot->key.type == APC_CACHE_KEY_FILE) { +#ifdef PHP_WIN32 + char buf[20]; + sprintf(buf, "%I64d", slot->key.data.file.device); + add_assoc_string(item->value, "device", buf, 1); +#else add_assoc_long(item->value, "device", slot->key.data.file.device); +#endif } } if (APC_ITER_INODE & iterator->format) { if(slot->key.type == APC_CACHE_KEY_FILE) { +#ifdef PHP_WIN32 + char buf[20]; + sprintf(buf, "%I64d", slot->key.data.file.device); + add_assoc_string(item->value, "device", buf, 1); +#else add_assoc_long(item->value, "inode", slot->key.data.file.inode); +#endif } } if (APC_ITER_KEY & iterator->format) { Index: apc_cache.c =================================================================== --- apc_cache.c (revision 289689) +++ apc_cache.c (working copy) @@ -48,10 +48,17 @@ static void apc_cache_expunge(apc_cache_t* cache, size_t size); /* {{{ hash */ +#ifdef PHP_WIN32 +static __int64 hash(apc_cache_key_t key) +{ + return key.data.file.device + key.data.file.inode; +} +#else static unsigned int hash(apc_cache_key_t key) { return key.data.file.device + key.data.file.inode; } +#endif /* }}} */ /* {{{ string_nhash_8 */ @@ -700,7 +707,7 @@ t = apc_time(); /* try to create a cache key; if we fail, give up on caching */ - if (!apc_cache_make_file_key(&key, filename, PG(include_path), t TSRMLS_CC)) { + if (!apc_cache_make_file_key(&key, filename, PG(include_path), t, NULL TSRMLS_CC)) { apc_wprint("Could not stat file %s, unable to delete from cache.", filename); return -1; } @@ -748,14 +755,23 @@ int apc_cache_make_file_key(apc_cache_key_t* key, const char* filename, const char* include_path, - time_t t + time_t t, + zend_file_handle* zfh TSRMLS_DC) { struct stat *tmp_buf=NULL; struct apc_fileinfo_t fileinfo = { {0}, }; int len; +#ifdef PHP_WIN32 + HANDLE hFile = INVALID_HANDLE_VALUE; + BY_HANDLE_FILE_INFORMATION hInfo; + BOOL close_hfile = FALSE; +#endif assert(key != NULL); + if (!filename && zfh && zfh->opened_path) { + filename = zfh->opened_path; + } if (!filename || !SG(request_info).path_translated) { #ifdef __DEBUG_APC__ @@ -832,8 +848,56 @@ return 0; } +#if PHP_WIN32 + if (zfh) { + switch (zfh->type) { + case ZEND_HANDLE_FD: + hFile = (HANDLE) _get_osfhandle(zfh->handle.fd); + break; + + case ZEND_HANDLE_FP: + hFile = (HANDLE) _get_osfhandle(fileno(zfh->handle.fp)); + break; + + default: + return 0; + } + } + + if (hFile == INVALID_HANDLE_VALUE) { + /* Use the FD from zend handle instead, get a file HANDLE using + _get_osfhandle */ + HANDLE hFile; + + hFile = CreateFile(filename, GENERIC_WRITE, FILE_SHARE_WRITE|FILE_SHARE_READ, NULL, OPEN_ALWAYS, FILE_FLAG_BACKUP_SEMANTICS, NULL); + if (!hFile) { +# ifdef __DEBUG_APC__ + fprintf(stderr,"Cannot create a file HANDLE for %s\n", filename); +# endif + return 0; + } + close_hfile = TRUE; + } + + if (!GetFileInformationByHandle(hFile, &hInfo)) { +# ifdef __DEBUG_APC__ + fprintf(stderr,"Cannot get file information from handle\n"); +# endif + CloseHandle(hFile); + return 0; + } + + if (close_hfile) { + CloseHandle(hFile); + } + key->data.file.device = hInfo.dwVolumeSerialNumber; + key->data.file.inode = (((apc_ino_t)(hInfo.nFileIndexHigh) << 32) | (apc_ino_t) hInfo.nFileIndexLow); + +#else key->data.file.device = fileinfo.st_buf.sb.st_dev; key->data.file.inode = fileinfo.st_buf.sb.st_ino; +#endif + /* * If working with content management systems that like to munge the mtime, * it might be appropriate to key off of the ctime to be immune to systems Index: apc_cache.h =================================================================== --- apc_cache.h (revision 289689) +++ apc_cache.h (working copy) @@ -49,6 +49,14 @@ #define APC_CACHE_KEY_USER 2 #define APC_CACHE_KEY_FPFILE 3 +#ifdef PHP_WIN32 +typedef __int64 apc_ino_t; +typedef __int64 apc_dev_t; +#else +typedef ino_t apc_ino_t; +typedef ino_t apc_dev_t; +#endif + /* {{{ cache locking macros */ #define CACHE_LOCK(cache) { LOCK(cache->header->lock); cache->has_lock = 1; } #define CACHE_UNLOCK(cache) { UNLOCK(cache->header->lock); cache->has_lock = 0; } @@ -62,8 +70,8 @@ typedef union _apc_cache_key_data_t { struct { - dev_t device; /* the filesystem device */ - ino_t inode; /* the filesystem inode */ + apc_dev_t device; /* the filesystem device */ + apc_ino_t inode; /* the filesystem inode */ } file; struct { const char *identifier; @@ -231,7 +239,8 @@ extern int apc_cache_make_file_key(apc_cache_key_t* key, const char* filename, const char* include_path, - time_t t + time_t t, + zend_file_handle* h TSRMLS_DC); /* @@ -259,8 +268,8 @@ typedef union _apc_cache_link_data_t { struct { char *filename; - dev_t device; - ino_t inode; + apc_ino_t device; + apc_dev_t inode; unsigned char *md5; } file; struct { Index: apc_bin.c =================================================================== --- apc_bin.c (revision 289689) +++ apc_bin.c (working copy) @@ -894,7 +894,7 @@ goto failure; } - if (!apc_cache_make_file_key(&cache_key, ep->val.file.filename, PG(include_path), t TSRMLS_CC)) { + if (!apc_cache_make_file_key(&cache_key, ep->val.file.filename, PG(include_path), t, NULL TSRMLS_CC)) { goto failure; }