Re: msdmo: Array parameter is passed to function as pointer so loses size information

[Robert Shearman]

Andrew Talbot wrote:
> @@ -291,8 +291,9 @@ lend:
>   *
>   * Get DMP Name from the registry
>   */
> -HRESULT WINAPI DMOGetName(REFCLSID clsidDMO, WCHAR szName[80])
> +HRESULT WINAPI DMOGetName(REFCLSID clsidDMO, WCHAR szName[])
>  {
> +#define NAME_SIZE   80  /* Size of szName[] */
>      WCHAR szguid[64];
>      HRESULT hres;
>      HKEY hrkey = 0;
> @@ -311,7 +312,7 @@ HRESULT WINAPI DMOGetName(REFCLSID clsidDMO, WCHAR szName[80])
>      if (ERROR_SUCCESS != hres)
>          goto lend;
>
> -    count = sizeof(szName);
> +    count = NAME_SIZE;
>      hres = RegQueryValueExW(hkey, NULL, NULL, NULL, 
>          (LPBYTE) szName, &count);

This is incorrect. count is the size in bytes of the buffer passed in 
(szName) and so should be sizeof(szName) not 
sizeof(szName)/sizeof(szName[0]) (i.e. 80).

I see this patch has already been committed, so 
a9200b24014607c4c82fb052b97de88daa804a81 should be reverted.

If you want to pick up errors like passing the wrong size into functions 
then I would suggest using an automatic checker that is able to use 
semantic information, like Microsoft's PREfast.

-- 
Rob Shearman

msdmo: Array parameter is passed to function as pointer so loses size information

[Andrew Talbot]

Robert Shearman wrote:
> This is incorrect. count is the size in bytes of the buffer passed in
> (szName) and so should be sizeof(szName) not
> sizeof(szName)/sizeof(szName[0]) (i.e. 80).

Are you sure? MSDN says "szName: Array of 80 Unicode characters that
receives the name of the DMO".
> If you want to pick up errors like passing the wrong size into functions
> then I would suggest using an automatic checker that is able to use
> semantic information, like Microsoft's PREfast.

Your implied presumption may not be correct. But, in any case, when arrays
are passed as arguments to functions they are converted to pointers, so
sizeof(szName) would represent the size of a pointer to the array, not the
size of the array itself.

-- 
Andy.

msdmo: Array parameter is passed to function as pointer so loses size information

[John Klehm]

On Wed, Apr 9, 2008 at 11:54 AM, Andrew Talbot
<Andrew.Talbot(a)talbotville.com> wrote:
> Robert Shearman wrote:
> > This is incorrect. count is the size in bytes of the buffer passed in
> > (szName) and so should be sizeof(szName) not
> > sizeof(szName)/sizeof(szName[0]) (i.e. 80).

Andrew T is right about arrays being decayed to pointers when passed
into functions, so sizeof(szName) is going to reuturn 4 or 8 or
whatever.  If count needs to be the size of the buffer shouldn't it
be:

count = NAME_SIZE * sizeof(WCHAR);

but probably better would be DMO_MAX_NAME_SIZE and be in a header
somewhere (dmo.h?)?

Regards,
John

msdmo: Array parameter is passed to function as pointer so loses size information

[Andrew Talbot]

John Klehm wrote:

If count needs to be the size of the buffer shouldn't it
> be:
>
> count = NAME_SIZE * sizeof(WCHAR);
>
> but probably better would be DMO_MAX_NAME_SIZE and be in a header
> somewhere (dmo.h?)?
>
> Regards,
> John

Ah yes, whoops. I'm pretty sure I had just that lined up, but I reckon I
must have done an 'undo' or something and taken off the multiplier. I'm
hesitant to create a header just for one constant, so I shall send a patch
to bring in the multiplier and with the more descriptive name, since
Alexandre rightly dropped my descriptive comment.

Thanks for catching that!

-- 
Andy.

msdmo: Array parameter is passed to function as pointer so loses size information

[Robert Shearman]

Andrew Talbot wrote:
> Robert Shearman wrote:
> > This is incorrect. count is the size in bytes of the buffer passed in
> > (szName) and so should be sizeof(szName) not
> > sizeof(szName)/sizeof(szName[0]) (i.e. 80).
>
> Are you sure? MSDN says "szName: Array of 80 Unicode characters that
> receives the name of the DMO".

It doesn't matter what MSDN says about szName, RegQueryValueExW still 
takes the size in bytes, not characters. I.e. count should be set to 
NAME_SIZE * sizeof(WCHAR), not NAME_SIZE.
> But, in any case, when arrays
> are passed as arguments to functions they are converted to pointers, so
> sizeof(szName) would represent the size of a pointer to the array, not the
> size of the array itself.

I didn't realise this before but the attached test program prints:
$ ./atest
sizeof(array) = 4
So you are right. Still, my point stands that the code is still passing 
in a smaller size than the size of the buffer and so is not correct.

-- 
Rob Shearman

msdmo: Array parameter is passed to function as pointer so loses size information

[Andrew Talbot]

Robert Shearman wrote:
> It doesn't matter what MSDN says about szName, RegQueryValueExW still
> takes the size in bytes, not characters. I.e. count should be set to
> NAME_SIZE * sizeof(WCHAR), not NAME_SIZE.

Hi Robert,

Yes, indeed. I believe I may have had an editing accident with that one.
> [...] [M]y point stands that the code is still passing
> in a smaller size than the size of the buffer and so is not correct.

You were indeed right on that and I was wrong. I only looked at what was
coming in and not at what was being done to it subsequently. Thanks for the
feedback.

-- 
Andy.