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]
