Re: cleanup a->w cross calls in advapi32/crypt.c

[Mike McCormack]

James Hawkins wrote:
> -
>    PWSTR keyname;
>
> - keyname = CRYPT_Alloc(strlen(KEYSTR) + strlen(pProvName) +1);
> -
>    keyname = CRYPT_Alloc((strlenW(KEYSTR) + strlenW(pProvName) + 1) * sizeof(WCHAR));
> if (keyname)
>  {
> -
> strcpy(keyname, KEYSTR);
> -
> strcpy(keyname + strlen(KEYSTR), pProvName);
> -
> strcpyW(keyname, KEYSTR);
> -
> strcpyW(keyname + strlenW(KEYSTR) * sizeof(WCHAR), pProvName);
> } else
> SetLastError(ERROR_NOT_ENOUGH_MEMORY);
> return keyname;
>
> }

This doesn't look right.... when using pointer arithmetic, you do so in 
increments of the pointer's type (eg. WCHAR). The following two lines 
are the same, and are both wrong:

strcpyW(keyname + strlenW(KEYSTR) * sizeof(WCHAR), pProvName);
   strcpyW(&keyname[strlenW(KEYSTR) * sizeof(WCHAR)], pProvName);

The right version would be:

strcpyW(&keyname[strlenW(KEYSTR)], pProvName);

but how about just using:

strcatW( keyname, pProvname );

Mike

cleanup a->w cross calls in advapi32/crypt.c

[James Hawkins]

> -
>         strcpy(keyname + strlen(KEYSTR), pProvName);

I changed that line to strcatW(keyname, pProvName).  That makes a lot
more sense Mike, thanks for the tip.

I'm wasn't exactly sure on this one so it would be great if you could
help me on this one.  When using pointer arithmetic, do the operations
such as --, ++... increment or decrement by the size of the pointer
type?  For example,

*(ptr - sizeof(WCHAR)) = (dwType % 10) + '0';
*(ptr - sizeof(WCHAR) * 2) = ((dwType / 10) % 10) + '0';
*(ptr - sizeof(WCHAR) * 3) = (dwType / 100) + '0';

Is the sizeof(WCHAR) multiplication redundant because --ptr actually
moves ptr down one WCHAR?  I understand that if that's the case, but
what about when the pointer is first initialized?

ptr = keyname + strlenW(keyname);

I guess it holds true here as well because keyname is a pointer and
we're adding ot it.  If that is the case, the included patch fixes the
two things mentioned.

On Mon, 02 Aug 2004 19:55:12 +0900, Mike McCormack  wrote:
> James Hawkins wrote:
> > -
> >    PWSTR keyname;
> >
> > -
> > keyname = CRYPT_Alloc(strlen(KEYSTR) + strlen(pProvName) +1);
> > -
> >    keyname = CRYPT_Alloc((strlenW(KEYSTR) + strlenW(pProvName) + 1) * sizeof(WCHAR));
> > if (keyname)
> > {
> > -
> >         strcpy(keyname, KEYSTR);
> > -
> >         strcpy(keyname + strlen(KEYSTR), pProvName);
> > -
> >         strcpyW(keyname, KEYSTR);
> > -
> >         strcpyW(keyname + strlenW(KEYSTR) * sizeof(WCHAR), pProvName);
> > } else
> >         SetLastError(ERROR_NOT_ENOUGH_MEMORY);
> > return keyname;
> >
> > }
>
> This doesn't look right.... when using pointer arithmetic, you do so in
> increments of the pointer's type (eg. WCHAR). The following two lines
> are the same, and are both wrong:
>
> strcpyW(keyname + strlenW(KEYSTR) * sizeof(WCHAR), pProvName);
>    strcpyW(&keyname[strlenW(KEYSTR) * sizeof(WCHAR)], pProvName);
>
> The right version would be:
>
> strcpyW(&keyname[strlenW(KEYSTR)], pProvName);
>
> but how about just using:
>
> strcatW( keyname, pProvname );
>
> Mike

cleanup a->w cross calls in advapi32/crypt.c

[Dmitry Timoshkov]

"James Hawkins"  wrote:
> ptr = keyname + strlenW(keyname);
>
> I guess it holds true here as well because keyname is a pointer and
> we're adding ot it.  If that is the case, the included patch fixes the
> two things mentioned.

You have to look for other similar, not mentioned cases, and fix them
as well.

cleanup a->w cross calls in advapi32/crypt.c

[Shachar Shemesh]

James Hawkins wrote:
> > -
> >         strcpy(keyname + strlen(KEYSTR), pProvName);
>
> I changed that line to strcatW(keyname, pProvName).  That makes a lot
> more sense Mike, thanks for the tip.
>
> I'm wasn't exactly sure on this one so it would be great if you could
> help me on this one.  When using pointer arithmetic, do the operations
> such as --, ++... increment or decrement by the size of the pointer
> type?  For example,
>
> *(ptr - sizeof(WCHAR)) = (dwType % 10) + '0';
> *(ptr - sizeof(WCHAR) * 2) = ((dwType / 10) % 10) + '0';
> *(ptr - sizeof(WCHAR) * 3) = (dwType / 100) + '0';
>
> Is the sizeof(WCHAR) multiplication redundant because --ptr actually
> moves ptr down one WCHAR?  I understand that if that's the case, but
> what about when the pointer is first initialized?
>
> ptr = keyname + strlenW(keyname);
>
> I guess it holds true here as well because keyname is a pointer and
> we're adding ot it.  If that is the case, the included patch fixes the
> two things mentioned.

Ok, whenever such matters come up, it's always wise to go to the source 
and ask the most authorative book there is. This is what the bible has 
to say about this (bible = "The C Programing Language, 2nd edition"):
Page 98 bottom:
"These remarks are true regardless of the type or size of the variables 
in the array a. The meaning of "adding 1 to a pointer," and by 
extension, all pointer arithmetics, is that pa+1 points to the next 
object, and pa+i points to the i-th object beyond pa."

In other words, ANY time you add or subtract and integer from a pointer, 
the result is to move the pointer to another array element. No need to 
perform these "word size" calculations anywhere.

Question: Did you compile and check you patch?

Shachar

cleanup a->w cross calls in advapi32/crypt.c

[James Hawkins]

I have compiled crypt and run the tests with no failures.
This latest patch removes all changes to the pointer arithmetic.

On Tue, 03 Aug 2004 07:55:51 +0300, Shachar Shemesh
 wrote:
> James Hawkins wrote:
> > > -
> > >         strcpy(keyname + strlen(KEYSTR), pProvName);
> >
> > I changed that line to strcatW(keyname, pProvName).  That makes a lot
> > more sense Mike, thanks for the tip.
> >
> > I'm wasn't exactly sure on this one so it would be great if you could
> > help me on this one.  When using pointer arithmetic, do the operations
> > such as --, ++... increment or decrement by the size of the pointer
> > type?  For example,
> >
> > *(ptr - sizeof(WCHAR)) = (dwType % 10) + '0';
> > *(ptr - sizeof(WCHAR) * 2) = ((dwType / 10) % 10) + '0';
> > *(ptr - sizeof(WCHAR) * 3) = (dwType / 100) + '0';
> >
> > Is the sizeof(WCHAR) multiplication redundant because --ptr actually
> > moves ptr down one WCHAR?  I understand that if that's the case, but
> > what about when the pointer is first initialized?
> >
> > ptr = keyname + strlenW(keyname);
> >
> > I guess it holds true here as well because keyname is a pointer and
> > we're adding ot it.  If that is the case, the included patch fixes the
> > two things mentioned.
>
> Ok, whenever such matters come up, it's always wise to go to the source
> and ask the most authorative book there is. This is what the bible has
> to say about this (bible = "The C Programing Language, 2nd edition"):
> Page 98 bottom:
> "These remarks are true regardless of the type or size of the variables
> in the array a. The meaning of "adding 1 to a pointer," and by
> extension, all pointer arithmetics, is that pa+1 points to the next
> object, and pa+i points to the i-th object beyond pa."
>
> In other words, ANY time you add or subtract and integer from a pointer,
> the result is to move the pointer to another array element. No need to
> perform these "word size" calculations anywhere.
>
> Question: Did you compile and check you patch?
>          Shachar
>
> --
> Shachar Shemesh
> Lingnu Open Source Consulting ltd.
> http://www.lingnu.com/