Re: [crypt32] CryptProtectData/CryptUnprotectData helper functions
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. -- Dimi Paun <[EMAIL PROTECTED]> Lattica, Inc.
Re: [crypt32] CryptProtectData/CryptUnprotectData helper functions
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
Re: [crypt32] CryptProtectData/CryptUnprotectData helper functions
Kees Cook <[EMAIL PROTECTED]> 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. -- Alexandre Julliard [EMAIL PROTECTED]
Re: [crypt32] CryptProtectData/CryptUnprotectData helper functions
On Wed, May 18, 2005 at 12:29:58PM -0700, Kees Cook wrote: > Take two. Ignore... this patch is broken, and I need to do an update of wine_dbg_printf() -> MESSAGE() -- Kees Cook@outflux.net
Re: [crypt32] CryptProtectData/CryptUnprotectData helper functions
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. -- Kees Cook@outflux.net
Re: [crypt32] CryptProtectData/CryptUnprotectData helper functions
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
Re: [crypt32] CryptProtectData/CryptUnprotectData helper functions
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? -- Kees Cook@outflux.net
Re: [crypt32] CryptProtectData/CryptUnprotectData helper functions
Kees Cook <[EMAIL PROTECTED]> 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. -- Alexandre Julliard [EMAIL PROTECTED]