Kees Cook kees@outflux.net writes:
+static int +hexprint(const char *s, unsigned char *p, int n) +{
- char report[80];
- int r=-1;
- snprintf(report,16,"%14s:", s);
- while (--n >= 0)
- {
if (r++ % 20 == 19)
{
wine_dbg_printf("%s\n",report);
snprintf(report,16,"%14s ", "");
}
sprintf(report+strlen(report)," %02x", *p++);
- }
- wine_dbg_printf("%s\n",report);
You should use wine_dbg_sprintf here and return a string.
+static +void serialize_dword(DWORD value,BYTE ** ptr) +{
- /*TRACE("called\n");*/
- *((DWORD*)*ptr)=value;
- *ptr+=sizeof(DWORD);
This (and other similar things later on) isn't safe for CPUs that don't allow unaligned accesses, you have to use memcpy instead.
+static +BOOL fill_protect_data(struct protect_data_t * pInfo, LPCWSTR szDataDescr,
HCRYPTPROV hProv)
+{
- DWORD dwStrLen;
- TRACE("called\n");
- if (!pInfo) return FALSE;
- dwStrLen=lstrlenW(szDataDescr);
- memset(pInfo,0,sizeof(*pInfo));
- pInfo->count0=0x0001;
- pInfo->info0.cbData=strlen(crypt_magic_str)+1;
- pInfo->info0.pbData=strdup(crypt_magic_str);
You don't want to use strdup, you should use HeapAlloc and friends (especially since you use HeapFree later on).
+static void +crypt_report_func_input(DATA_BLOB* pDataIn,
DATA_BLOB* pOptionalEntropy,
CRYPTPROTECT_PROMPTSTRUCT* pPromptStruct,
DWORD dwFlags)
+{
- wine_dbg_printf("\tpPromptStruct: 0x%x\n",(unsigned int)pPromptStruct);
- if (pPromptStruct)
- {
wine_dbg_printf("\t\tcbSize: 0x%x\n",(unsigned int)pPromptStruct->cbSize);
wine_dbg_printf("\t\tdwPromptFlags: 0x%x\n",(unsigned int)pPromptStruct->dwPromptFlags);
wine_dbg_printf("\t\thwndApp: 0x%x\n",(unsigned int)pPromptStruct->hwndApp);
wine_dbg_printf("\t\tszPrompt: 0x%x %s\n",
(unsigned int)pPromptStruct->szPrompt,
pPromptStruct->szPrompt ? debugstr_w(pPromptStruct->szPrompt)
: "");
- }
- wine_dbg_printf("\tdwFlags: 0x%04x\n",(unsigned int)dwFlags);
- wine_dbg_printf("\tpDataIn->cbData: %u\n",(unsigned int)pDataIn->cbData);
- wine_dbg_printf("\tpDataIn->pbData: 0x%x\n",(unsigned int)pDataIn->pbData);
- hexprint("pbData", pDataIn->pbData, pDataIn->cbData);
- if (pOptionalEntropy)
- {
wine_dbg_printf("\tpOptionalEntropy->cbData: %u\n",(unsigned int)pOptionalEntropy->cbData);
wine_dbg_printf("\tpOptionalEntropy->pbData: 0x%x\n",(unsigned int)pOptionalEntropy->pbData);
hexprint("pbData", pOptionalEntropy->pbData, pOptionalEntropy->cbData);
wine_dbg_printf("\t\t%s\n",debugstr_an(pOptionalEntropy->pbData,pOptionalEntropy->cbData));
- }
+}
+static void +announce_bad_opaque_data() +{
- wine_dbg_printf("CryptUnprotectData received the following pDataIn DATA_BLOB that seems to\n");
- wine_dbg_printf("have NOT been generated by Wine:\n");
+}
You should use the TRACE/FIXME macros here, not raw wine_dbg_printf.
On Wed, May 18, 2005 at 07:31:03PM +0200, Alexandre Julliard wrote:
- wine_dbg_printf("%s\n",report);
You should use wine_dbg_sprintf here and return a string.
Okay.
+static +void serialize_dword(DWORD value,BYTE ** ptr) +{
- /*TRACE("called\n");*/
- *((DWORD*)*ptr)=value;
- *ptr+=sizeof(DWORD);
This (and other similar things later on) isn't safe for CPUs that don't allow unaligned accesses, you have to use memcpy instead.
Ah, whoops. I will fix these.
- pInfo->info0.pbData=strdup(crypt_magic_str);
You don't want to use strdup, you should use HeapAlloc and friends (especially since you use HeapFree later on).
Is there a HeapAlloc-using version of strdup? I see a while back someone implemented their own in their own code; is there a common batch of wine functions, or should I do the same thing?
http://www.winehq.org/hypermail/wine-patches/2004/08/0558.html
StrDup is defined for Windows in .NET and VB, but not for VC, it seems?
+crypt_report_func_input(DATA_BLOB* pDataIn,
DATA_BLOB* pOptionalEntropy,
CRYPTPROTECT_PROMPTSTRUCT* pPromptStruct,
DWORD dwFlags)
+{
- wine_dbg_printf("\tpPromptStruct: 0x%x\n",(unsigned int)pPromptStruct);
+static void +announce_bad_opaque_data() +{
- wine_dbg_printf("CryptUnprotectData received the following pDataIn DATA_BLOB that seems to\n");
- wine_dbg_printf("have NOT been generated by Wine:\n");
+}
You should use the TRACE/FIXME macros here, not raw wine_dbg_printf.
FIXME is sane for the "announce_bad_opaque_data", but I'd still like to use something that doesn't prefix the hexdumps with the function name for easier readability in the crypt_report_func_input, since there is already a TRACE/FIXME call being made prior to it's call. Is this okay?
On Wed, 18 May 2005 12:08:35 -0700, Kees Cook wrote:
FIXME is sane for the "announce_bad_opaque_data", but I'd still like to use something that doesn't prefix the hexdumps with the function name for easier readability in the crypt_report_func_input, since there is already a TRACE/FIXME call being made prior to it's call. Is this okay?
It's usually OK to use MESSAGE for that, but if it's a message users might be seeing often it's best to keep it as a WARN or TRACE.
thanks -mike
On Wed, May 18, 2005 at 08:44:17PM +0100, Mike Hearn wrote:
It's usually OK to use MESSAGE for that, but if it's a message users might be seeing often it's best to keep it as a WARN or TRACE.
Ah-ha, yeah. I should use MESSAGE for that. It's only going to appear if peopl have already turned on trace/warn, or immediately following a FIXME that includes a path. I just want to use it for the readability of the structure.
On Wed, 18 May 2005 13:34:39 -0700, Kees Cook wrote:
Ah-ha, yeah. I should use MESSAGE for that. It's only going to appear if peopl have already turned on trace/warn, or immediately following a FIXME that includes a path. I just want to use it for the readability of the structure.
Actually, MESSAGEs always appear. They're meant for end user critical information rather than developer info. If it's developer info either use TRACE/WARN or do:
if (TRACE_ON(crypt)) MESSAGE("foo\n");
but I think AJ prefers the former ;)
thanks -mike
On Wed, 2005-05-18 at 22:57 +0100, Mike Hearn wrote:
if (TRACE_ON(crypt)) MESSAGE("foo\n");
There's no reason to use this, ever. Just use the regular FIXME/ERR/WARN/TRACE() as appropriate.
Kees Cook kees@outflux.net writes:
FIXME is sane for the "announce_bad_opaque_data", but I'd still like to use something that doesn't prefix the hexdumps with the function name for easier readability in the crypt_report_func_input, since there is already a TRACE/FIXME call being made prior to it's call. Is this okay?
No, you really want the prefix on all lines, otherwise it makes a big mess when multiple threads are printing things at the same time.