A bug in the Content plugin api ?

[ghisler(Author)]

You mean for ft_numeric_floating? The length (in bytes) of the entire return value is given by the maxlen parameter. Unicode is expected because of the "W" in the function name.

[tbeu]

You mean for ft_numeric_floating?

Right.
I see. I still miss both information in the plugin documentation.

[tbeu]

There is a (documentation) problem with the data type of the optional trailing string for ft_numeric_floating. ContentGetValue expects a normal string whereas ContentGetValueW expects a wide string. For Unicode support the programming recommondation is to call ContentGetValueW from ContentGetValue, i.e.:

Code: Select all

int __stdcall ContentGetValue(char* FileName, int FieldIndex, int UnitIndex, void* FieldValue, int maxlen, int flags)
{
	wchar_t FileNameW[wdirtypemax];
	return ContentGetValueW(awfilenamecopy(FileNameW, FileName), FieldIndex, UnitIndex, FieldValue, maxlen, flags);
}

This is wrong for ft_numeric_floating where the trailing string of FieldValue should be back converted from wide string to normal string.

[MVV]

It seems that ContentGetValue function should check value returned by ContentGetValueW and perform corrections if needed.

Code: Select all

int wcstombs(void* ptr, size_t maxbytes) {
	char* dst=(char*)ptr;
	std::auto_ptr<wchar_t> src(wcsdup((wchar_t*)ptr));
	int r=wcstombs(src.get(), dst, maxbytes);
	if (maxbytes) dst[maxbytes-1]=0;
	return r;
}

int __stdcall ContentGetValue(char* FileName, int FieldIndex, int UnitIndex, void* FieldValue, int maxlen, int flags) { 
	wchar_t FileNameW[wdirtypemax]; 
	int r=ContentGetValueW(awfilenamecopy(FileNameW, FileName), FieldIndex, UnitIndex, FieldValue, maxlen, flags);
	if (r==ft_numeric_floating) {
		wcstombs((char*)FieldValue+8, maxlen-8);
	}
	return r;
}

[tbeu]

It seems that ContentGetValue function should check value returned by ContentGetValueW and perform corrections if needed.

Yes, absolutely. Your code does not compile, but this works for me:

Code: Select all

int __stdcall ContentGetValue(char* FileName, int FieldIndex, int UnitIndex, void* FieldValue, int maxlen, int flags)
{
	wchar_t FileNameW[wdirtypemax];
	int retVal = ContentGetValueW(awfilenamecopy(FileNameW, FileName), FieldIndex, UnitIndex, FieldValue, maxlen, flags);
	if (retVal == ft_numeric_floating)
	{
		wchar_t* p = _wcsdup((const wchar_t*)((char*)FieldValue + sizeof(double)));
		walcopy((char*)FieldValue + sizeof(double), p, maxlen - sizeof(double) - 1);
		free(p);
	}
	return retVal;
}

I hope that Chr. Ghisler will add a fix to contentplugin.HLP.

[MVV]

Sorry, I doesn't try to compile it.
Well, I messed up first two parameters of standard wcstombs (should be dst, src.get() instead of src.get(), dst) - usually I use WideCharToMultiByte instead (it is system function and doesn't require any CRT code).

[tbeu]

usually I use WideCharToMultiByte instead

This is exactly what walcopy (from above code line) calls.

[MVV]

I see.

BTW usually I prefer to specify buffer size instead of length (i.e. length INCLUDING terminating zero byte). This way causes no mix of buffer size and string length.

Code: Select all

char* walcopy1(char* outname,WCHAR* inname,int size)
{
	if (inname) {
		WideCharToMultiByte(CP_ACP,0,inname,-1,outname,size,NULL,NULL);
		outname[size-1]=0;
		return outname;
	} else
		return NULL;
}

Function WideCharToMultiByte accepts buffer size too (not string length).

ContentGetValue accepts buffer size too although parameter name tells 'length' and not 'size', and we must remember it.

I always use 'sz' or 'size' for buffer sizes and 'len' or 'length' for string lengths (w/o null terminator). It reminds how functions work w/o looking their code.

[tbeu]

Function WideCharToMultiByte accepts buffer size too (not string length) like ContentGetValue.

Yes, you are right. The functions walcopy and awlcopy of cunicode.cpp (as part of official sample wfx) need to be corrected.

I'm always use 'sz' or 'size' for buffer sizes and 'len' or 'length' for string lengths (w/o null terminator). It reminds how functions work w/o looking its code.

Well, I might adopt it.

[tbeu]

ContentGetValue accepts buffer size too.

This is also true for ContentGetSupportedField and ContentEditValue, whereas ContentGetDetectString takes max. string length as argument.

[MVV]

Right. And sometimes it is hard to find out which value should be expected (documentation sometimes doesn't mention such things too).

BTW I would prefer to call parameter name for ContentGetValue and ContentEditValue as MaxBytes to accent that it accepts number of bytes and not number of characters.

Also I would use const modifier for read-only strings like filenames. Usually if I see non-const pointer to some buffer/struct, I try to find what kind of data I should write to it. Also sometimes parameters are input and output simultaneously (e.g. in WFX functions FsGetLocalName, FsExtractCustomIcon, FsExecuteFile).

So, correct function prototypes for WDX are:

int __stdcall ContentGetDetectString(char* DetectString, int MaxChars);

int __stdcall ContentGetSupportedField(int FieldIndex, char* FieldName, char* Units, int MaxChars);
int __stdcall ContentGetValue(const char* FileName, int FieldIndex, int UnitIndex, void* FieldValue, int MaxBytes, int Flags);
int __stdcall ContentGetValueW(const wchar_t* FileName, int FieldIndex, int UnitIndex, void* FieldValue, int MaxBytes, int Flags);

void __stdcall ContentSetDefaultParams(ContentDefaultParamStruct* DPS);
void __stdcall ContentPluginUnloading();

void __stdcall ContentStopGetValue(const char* FileName);
void __stdcall ContentStopGetValueW(const wchar_t* FileName);

int __stdcall ContentGetDefaultSortOrder(int FieldIndex);
int __stdcall ContentGetSupportedFieldFlags(int FieldIndex);

int __stdcall ContentSetValue(const char* FileName, int FieldIndex, int UnitIndex, int FieldType, void* FieldValue, int Flags);
int __stdcall ContentSetValueW(const wchar_t* FileName, int FieldIndex, int UnitIndex, int FieldType, void* FieldValue, int Flags);

int __stdcall ContentEditValue(HWND ParentWin, int FieldIndex, int UnitIndex, int FieldType, void* FieldValue, int MaxBytes, int Flags, const char* LangId);

void __stdcall ContentSendStateInformation(int State, const char* Path);
void __stdcall ContentSendStateInformationW(int State, const wchar_t* Path);

int __stdcall ContentCompareFiles(PROGRESSCALLBACKPROC ProgressCallback, int CompareIndex, const char* FileName1, const char* FileName2, FileDetailsStruct* FileDetails);
int __stdcall ContentCompareFilesW(PROGRESSCALLBACKPROC ProgressCallback, int CompareIndex, const wchar_t* FileName1, const wchar_t* FileName2, FileDetailsStruct* FileDetails);

It is quite useful to have such details: if you see MaxChars, it is buffer size in characters, if you see MaxBytes, it is buffer size in Bytes (so you should divide it by 2 for Unicode), if you see MaxLen, it is max number of nonzero characters you can have in buffer. Also, if some string/structure/buffer is defined as const, you always know that it is an input parameter.


Another example is callback function RequestProc in WFX API.

maxlen: Maximum length allowed for ReturnedText. The pointer ReturnedText must point to a buffer which can hold at least maxlen characters.

So it doesn't clarify is terminator counted or not. Only using debugging I saw that it doesn't (I set maxlen to 10 and function returned 10 characters + terminator; furthermore, TC even set max EDIT string length to this value) - it will cause buffer overrun if I pass full buffer size. So, parameter name is correct here, but again, it is better to pass buffer sizes instead of string lengths for input buffers.

So, prototypes for WFX API:

// callback functions
typedef int (__stdcall* tProgressProc)(int PluginNr, const char* SourceName, const char* TargetName, int PercentDone);
typedef int (__stdcall* tProgressProcW)(int PluginNr, const wchar_t* SourceName, const wchar_t* TargetName, int PercentDone);
typedef void (__stdcall* tLogProc)(int PluginNr, int MsgType, const char* LogString);
typedef void (__stdcall* tLogProcW)(int PluginNr, int MsgType, const wchar_t* LogString);

typedef BOOL (__stdcall* tRequestProc)(int PluginNr, int RequestType, const char* CustomTitle, const char* CustomText, char* ReturnedText, int MaxLen);
typedef BOOL (__stdcall* tRequestProcW)(int PluginNr, int RequestType, const wchar_t* CustomTitle, const wchar_t* CustomText, wchar_t* ReturnedText, int MaxLen);
typedef int (__stdcall* tCryptProc)(int PluginNr, int CryptoNr, int Mode, const char* ConnectionName, char* Password, int MaxLen);
typedef int (__stdcall* tCryptProcW)(int PluginNr, int CryptoNr, int Mode, const wchar_t* ConnectionName, wchar_t* Password, int MaxLen);

// Function prototypes
int __stdcall FsInit(int PluginNr, tProgressProc pProgressProc, tLogProc pLogProc, tRequestProc pRequestProc);
int __stdcall FsInitW(int PluginNr, tProgressProcW pProgressProcW, tLogProcW pLogProcW, tRequestProcW pRequestProcW);
void __stdcall FsSetCryptCallback(tCryptProc pCryptProc, int CryptoNr, int Flags);
void __stdcall FsSetCryptCallbackW(tCryptProcW pCryptProcW, int CryptoNr, int Flags);

HANDLE __stdcall FsFindFirst(const char* Path, WIN32_FIND_DATAA* FindData);
HANDLE __stdcall FsFindFirstW(const wchar_t* Path, WIN32_FIND_DATAW* FindData);
BOOL __stdcall FsFindNext(HANDLE Hdl, WIN32_FIND_DATAA* FindData);
BOOL __stdcall FsFindNextW(HANDLE Hdl, WIN32_FIND_DATAW* FindData);
int __stdcall FsFindClose(HANDLE Hdl);

BOOL __stdcall FsMkDir(const char* Path);
BOOL __stdcall FsMkDirW(const wchar_t* Path);

int __stdcall FsExecuteFile(HWND MainWin, char* RemoteName, const char* Verb);
int __stdcall FsExecuteFileW(HWND MainWin, wchar_t* RemoteName, const wchar_t* Verb);

int __stdcall FsRenMovFile(const char* OldName, const char* NewName, BOOL Move, BOOL OverWrite, const RemoteInfoStruct* RI);
int __stdcall FsRenMovFileW(const wchar_t* OldName, const wchar_t* NewName, BOOL Move, BOOL OverWrite, const RemoteInfoStruct* RI);

int __stdcall FsGetFile(const char* RemoteName, const char* LocalName, int CopyFlags, const RemoteInfoStruct* RI);
int __stdcall FsGetFileW(const wchar_t* RemoteName, const wchar_t* LocalName, int CopyFlags, const RemoteInfoStruct* RI);

int __stdcall FsPutFile(const char* LocalName, const char* RemoteName, int CopyFlags);
int __stdcall FsPutFileW(const wchar_t* LocalName, const wchar_t* RemoteName, int CopyFlags);

BOOL __stdcall FsDeleteFile(const char* RemoteName);
BOOL __stdcall FsDeleteFileW(const wchar_t* RemoteName);

BOOL __stdcall FsRemoveDir(const char* RemoteName);
BOOL __stdcall FsRemoveDirW(const wchar_t* RemoteName);

BOOL __stdcall FsDisconnect(const char* DisconnectRoot);
BOOL __stdcall FsDisconnectW(const wchar_t* DisconnectRoot);

BOOL __stdcall FsSetAttr(const char* RemoteName, int NewAttr);
BOOL __stdcall FsSetAttrW(const wchar_t* RemoteName, int NewAttr);

BOOL __stdcall FsSetTime(const char* RemoteName, const FILETIME* CreationTime, const FILETIME* LastAccessTime, const FILETIME* LastWriteTime);
BOOL __stdcall FsSetTimeW(const wchar_t* RemoteName, const FILETIME* CreationTime, const FILETIME* LastAccessTime, const FILETIME* LastWriteTime);

void __stdcall FsStatusInfo(const char* RemoteDir, int InfoStartEnd, int InfoOperation);
void __stdcall FsStatusInfoW(const wchar_t* RemoteDir, int InfoStartEnd, int InfoOperation);

void __stdcall FsGetDefRootName(char* DefRootName, int MaxChars);

int __stdcall FsExtractCustomIcon(char* RemoteName, int ExtractFlags, HICON* TheIcon);
int __stdcall FsExtractCustomIconW(wchar_t* RemoteName, int ExtractFlags, HICON* TheIcon);

void __stdcall FsSetDefaultParams(const FsDefaultParamStruct* DPS);

int __stdcall FsGetPreviewBitmap(char* RemoteName, int Width, int Height, HBITMAP* ReturnedBitmap);
int __stdcall FsGetPreviewBitmapW(wchar_t* RemoteName, int Width, int Height, HBITMAP* ReturnedBitmap);

BOOL __stdcall FsLinksToLocalFiles();

BOOL __stdcall FsGetLocalName(char* RemoteName, int MaxChars);
BOOL __stdcall FsGetLocalNameW(wchar_t* RemoteName, int MaxChars);

int __stdcall FsContentGetSupportedField(int FieldIndex, char* FieldName, char* Units, int MaxChars);
int __stdcall FsContentGetValue(const char* FileName, int FieldIndex, int UnitIndex, void* FieldValue, int MaxBytes, int Flags);
int __stdcall FsContentGetValueW(const wchar_t* FileName, int FieldIndex, int UnitIndex, void* FieldValue, int MaxBytes, int Flags);

void __stdcall FsContentPluginUnloading();

void __stdcall FsContentStopGetValue(const char* FileName);
void __stdcall FsContentStopGetValueW(const wchar_t* FileName);

int __stdcall FsContentGetDefaultSortOrder(int FieldIndex);
int __stdcall FsContentGetSupportedFieldFlags(int FieldIndex);

int __stdcall FsContentSetValue(const char* FileName, int FieldIndex, int UnitIndex, int FieldType, void* FieldValue, int Flags);
int __stdcall FsContentSetValueW(const wchar_t* FileName, int FieldIndex, int UnitIndex, int FieldType, void* FieldValue, int Flags);

BOOL __stdcall FsContentGetDefaultView(char* ViewContents, char* ViewHeaders, char* ViewWidths, char* ViewOptions, int MaxLen);
BOOL __stdcall FsContentGetDefaultViewW(wchar_t* ViewContents, wchar_t* ViewHeaders, wchar_t* ViewWidths, wchar_t* ViewOptions, int MaxLen);

int __stdcall FsGetBackgroundFlags();

Please correct me if I'm wrong in some definitions.

[tbeu]

ContentGetSupportedField and FsContentGetSupportedField take MaxBytes, too.

[MVV]

Theese work with strings (while ContentGetValue work with binary buffer) and exist only in ANSI where sizeof(char)==sizeof(byte). Also, documentation says:

maxlen: The maximum number of characters, including the trailing 0, which may be returned in each of the fields.

So I think MaxChars is more logical here (I use MaxChars for all string buffers).