Windows 2008

Fixed in development branch svn://svn.zabbix.com/branches/dev/ZBX-7856

(1) It is a bit unclear where "struct stat" should be used and where the new "zbx_stat_t" should be.

For instance, __zbx_zabbix_log() uses "zbx_stat_t" for a call to stat(), but src/libs/zbxsysinfo/common/file.c uses it the other way round: "struct stat" for a call to zbx_stat().

It seems that the stat()-ing interface should be made more intuitive.

While at it, please see r43975.

wiper It should be used at least in code that is used also on windows. But I missed the sysinfo/common/ directory.

wiper RESOLVED in r43980

asaveljevs These are our current definitions for zbx_stat():

#if defined(_WINDOWS)
#ifdef _UNICODE
#	define zbx_stat(path, buf)	__zbx_stat(path, buf)
#else
#	define zbx_stat(path, buf)	_stat64(path, buf)
#endif
#else
#	define zbx_stat(path, buf)	stat(path, buf)
#endif

The first definition accepts "zbx_stat_t", the second "struct __stat64", the third "struct stat".

Here are our current definitions for "zbx_stat_t":

#if defined(_WINDOWS) && defined(_UNICODE)
typedef struct __stat64	zbx_stat_t;
int	__zbx_stat(const char *path, zbx_stat_t *buf);
#else
typedef struct stat	zbx_stat_t;
#endif	/* _WINDOWS && _UNICODE */

It seems that these definitions do not match the second case above, because "zbx_stat_t" will be "struct stat", but _stat64() expects "struct __stat64".

asaveljevs Also, as pointed above, __zbx_zabbix_log() uses "zbx_stat_t" for a call to stat(). For Windows and Unicode, "zbx_stat_t" will be "struct __stat64". Can stat() really be called on that structure? REOPENED.

wiper Actually it would call _stat64 because of a define we had in zbxtypes (and now I understood why 'struct stat' worked with _wstat64 before). But it's really confusing, so I removed this defined and replaced all stat() calls used in Windows code with zbx_stat().

wiper RESOLVED in r44022

asaveljevs Looks good, but please see 44027.

wiper Thanks, CLOSED

(2) It might be that using _wstat64() is more efficient that opening a file, doing _fstat64() and closing the file.

Since symlinks are probably not used very often, would it make sense to only use the _fstat64() sequence if _wstat64() fails?

For instance, http://msdn.microsoft.com/en-us/library/14h5k7ff.aspx says "In these cases, _wstat will always report a file size of 0". Can we fall back to _fstat64() if _wstat64() reports a file size of 0?

wiper Yes, that sounds like a good idea. RESOLVED in r44043

asaveljevs Looks good, but please see r44062. It replaces close() with _close(). RESOLVED.

asaveljevs Actually, it does not seem to be that simple. The new code (doing _wstat64() first, then falling back to _fstat64()) does not work when there is a symlinked directory in the path. In that case, _wstat64() returns -1, saying that there is "No such file or directory". See "C:\zabbix\symlinks\" directory for test cases. REOPENED.

wiper It appears that directory was symlinked without /D switch. Could you please try it again?

asaveljevs Might be, the issue is not reproducible anymore. CLOSED.

(3) In src/libs/zbxsysinfo/linux/proc.c, there is a function open_proc_file():

static FILE	*open_proc_file(const char *filename)
{
	zbx_stat_t	s;

	if (0 != zbx_stat(filename, &s))
		return NULL;

	return fopen(filename, "r");
}

It might be that stat() is unnecessary here. Hence, open_proc_file() can be replaced with fopen().

wiper Nobody had a good idea why it was there, so removed it.
RESOLVED in r44033

asaveljevs The code did not compile. Fixed that in r44061. RESOLVED.

wiper thanks, CLOSED

(4) Should "/D UNICODE" be removed from Windows makefiles or is it a system define?

wiper UNICODE is required to use wide character version of Windows API, _UNICODE is required for tchar macros - TEXT(), _tcs functions. I understand that Windows defines UNICODE if _UNICODE is defined, so in theory we might omit it.

asaveljevs This seems to say that the safest way is to define both: http://www.winehq.org/pipermail/wine-devel/2002-January/003565.html . So let's leave as is. CLOSED.

(5) Please also see r44063. This fixes some warnings (unrelated to this task) on Windows.

wiper Good, CLOSED

Released in:
pre-2.0.12rc1 r44211
pre-2.2.4rc1 r44213
pre-2.3.0 r44214

(6) There is a call to _wstat() in src/libs/zbxconf/cfg.c in trunk that did not exist in 2.0 and was thus missed when merging from 2.0 into trunk.

The problem with that call is that it cannot be replaced with the current version of zbx_stat() in a straightforward way, because current zbx_stat() fails on directories: _wstat64() is successful, "buf->st_size" is zero, then _wopen() fails.

wiper RESOLVED in r44897

asaveljevs Looks almost good, except one initialization was missing. Please see r44898 before merging. RESOLVED.

wiper Thanks, CLOSED

(7) Cannot compile 2.0 zabbix_sender and zabbix_get.

log.o : error LNK2019: unresolved external symbol ___zbx_stat referenced in function ___zbx_zabbix_log ..\..\..\bin\win32\zabbix_sender.exe : fatal error LNK1120: 1 unresolved externals
NMAKE : fatal error U1077: '"C:\Program Files\Microsoft Visual Studio 9.0\VC\Bin\link.exe"' : return code '0x460'

wiper Fixed in development branch svn://svn.zabbix.com/branches/dev/ZBX-7856_2.0
RESOLVED

sasha CLOSED

wiper Released in:
pre-2.0.12rc1 r44887
pre-2.2.4rc1 44888
pre-2.3.0 r44889

The (6) fix is released in:
pre-2.3.0 r44923

(8) zbx_stat() is used for directories (logrt[]) also in 2.0. So the (6) zbx_stat() related fix must be backported to 2.0

wiper RESOLVED in r45103

asaveljevs Looks good. Works on symlinked directories, too. CLOSED.

Released in:
pre-2.0.12rc2 r45110
pre-2.2.4rc1 r45111
pre-2.3.0 r44923