Re: [PATCH v2 08/10] combase: Implement creation and deletion of HSTRING objects.

[Sebastian Lackner]

On 08.12.2014 10:26, Martin Storsjo wrote:
> +        return E_INVALIDARG;
> +    if (str == NULL)
> +    {
> +         *out = NULL;        <---
> +         return S_OK;        <---
> +    }
> +    if (priv->reference)
> +        return WindowsCreateString(priv->buffer, priv->length, out);

Wrong indentation.

Moreover I noticed a mistake in patch 10. I had the guess that preallocating an empty string should also return a NULL string-buffer.

+    /* Test creation of an empty buffer */
+    ok(pWindowsPreallocateStringBuffer(0, &ptr, &buf) == S_OK, "Failed to preallocate string buffer\n");
+    ok(buf != NULL, "Empty string buffer is a null string\n");
+    ok(ptr != NULL, "Empty string didn't return a buffer pointer\n");
+    ok(pWindowsDeleteStringBuffer(buf) == S_OK, "Failed to delete string buffer\n");
+

The testbot seems to confirm that:
https://newtestbot.winehq.org/JobDetails.pl?Key=10687

( I would suggest to wait with resending till after the daily commits, everything from patch 1-7 should be fine, and it probably doesn't make sense to resend the whole series several times. ;) )

Regards,
Sebastian

[PATCH v2 08/10] combase: Implement creation and deletion of HSTRING objects.

[Martin Storsjö]

On Mon, 8 Dec 2014, Sebastian Lackner wrote:
> On 08.12.2014 10:26, Martin Storsjo wrote:
> > +        return E_INVALIDARG;
> > +    if (str == NULL)
> > +    {
> > +         *out = NULL;        <---
> > +         return S_OK;        <---
> > +    }
> > +    if (priv->reference)
> > +        return WindowsCreateString(priv->buffer, priv->length, out);
>
> Wrong indentation.

Thanks for catching
> Moreover I noticed a mistake in patch 10. I had the guess that 
> preallocating an empty string should also return a NULL string-buffer.
>
> +    /* Test creation of an empty buffer */
> +    ok(pWindowsPreallocateStringBuffer(0, &ptr, &buf) == S_OK, "Failed to preallocate string buffer\n");
> +    ok(buf != NULL, "Empty string buffer is a null string\n");
> +    ok(ptr != NULL, "Empty string didn't return a buffer pointer\n");
> +    ok(pWindowsDeleteStringBuffer(buf) == S_OK, "Failed to delete string buffer\n");
> +
>
> The testbot seems to confirm that:
> https://newtestbot.winehq.org/JobDetails.pl?Key=10687

Ok, I fixed this locally, including a testcase for promoting an empty (aka 
null) string buffer into an empty (and also null) string.
> ( I would suggest to wait with resending till after the daily commits, 
> everything from patch 1-7 should be fine, and it probably doesn't make 
> sense to resend the whole series several times. ;) )

Ok, will do.

// Martin

[PATCH v2 08/10] combase: Implement creation and deletion of HSTRING objects.

[Martin Storsjö]

On Mon, 8 Dec 2014, Martin Storsjö wrote:
> On Mon, 8 Dec 2014, Sebastian Lackner wrote:
> > ( I would suggest to wait with resending till after the daily commits, 
> > everything from patch 1-7 should be fine, and it probably doesn't make 
> > sense to resend the whole series several times. ;) )
>
> Ok, will do.

I resubmitted the patchset with the ones that weren't committed (4-10 from 
the previous series, skipping the makedep one which isn't strictly 
necessary any longer, and was marked "pending" in the patches list).

// Martin