[PATCH 3/7] wsdapi: Build and write Types list for Hello message.

[Owen Rudge]

Signed-off-by: Owen Rudge <orudge(a)codeweavers.com>
---
   dlls/wsdapi/soap.c | 50 
++++++++++++++++++++++++++++++++++++++++++++++++++
   1 file changed, 50 insertions(+)

[Huw Davies]

On Fri, Apr 20, 2018 at 08:18:19AM +0100, Owen Rudge wrote:
> Signed-off-by: Owen Rudge <orudge(a)codeweavers.com>
> ---
>   dlls/wsdapi/soap.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 50 insertions(+)

> diff --git a/dlls/wsdapi/soap.c b/dlls/wsdapi/soap.c
> index af776048cb..68e6a32f73 100644
> --- a/dlls/wsdapi/soap.c
> +++ b/dlls/wsdapi/soap.c
> @@ -77,6 +77,7 @@ static const WCHAR bodyString[] = { 'B','o','d','y', 0 };
>  static const WCHAR helloString[] = { 'H','e','l','l','o', 0 };
>  static const WCHAR endpointReferenceString[] = { 'E','n','d','p','o','i','n','t','R','e','f','e','r','e','n','c','e', 0 };
>  static const WCHAR addressString[] = { 'A','d','d','r','e','s','s', 0 };
> +static const WCHAR typesString[] = { 'T','y','p','e','s', 0 };
>
> struct discovered_namespace
>  {
> @@ -499,6 +500,41 @@ static BOOL add_discovered_namespace(struct list *namespaces, WSDXML_NAMESPACE *
>      return TRUE;
>  }
>
> +static HRESULT build_types_list(LPWSTR buffer, size_t buffer_size, const WSD_NAME_LIST *list, struct list *namespaces)
> +{
> +    WCHAR format_string[] = { '%', 's', ':', '%', 's', ' ', 0 };
> +    LPWSTR current_buf_pos = buffer;
> +    size_t memory_needed = 0;
> +    const WSD_NAME_LIST *cur = list;
> +
> +    ZeroMemory(buffer, buffer_size);
> +
> +    while (cur != NULL)
> +    {
> +        /* Calculate space needed, including NULL character, colon and potential trailing space */
> +        memory_needed = sizeof(WCHAR) * (lstrlenW(cur->Element->LocalName) +
> +            lstrlenW(cur->Element->Space->PreferredPrefix) + 3);
> +
> +        if (current_buf_pos + memory_needed > buffer + buffer_size)
> +            return E_INVALIDARG;
> +
> +        current_buf_pos += wsprintfW(current_buf_pos, format_string, cur->Element->Space->PreferredPrefix,
> +            cur->Element->LocalName);
> +
> +        /* Record the namespace in the discovered namespaces list */
> +        if (!add_discovered_namespace(namespaces, cur->Element->Space))
> +            return E_FAIL;
> +
> +        cur = cur->Next;
> +    }
> +
> +    /* Remove the last trailing space */
> +    current_buf_pos--;
> +    *current_buf_pos = 0;

Is it possible that list could be NULL so that the while loop is never
executed?  If so, then the above line would touch buffer[-1].  If
list is never NULL consider rewriting the loop as a do-while.
> +
> +    return S_OK;
> +}
> +
>  static HRESULT duplicate_element(WSDXML_ELEMENT *parent, const WSDXML_ELEMENT *node, struct list *namespaces)
>  {
>      WSDXML_ATTRIBUTE *cur_attribute, *new_attribute, *last_attribute = NULL;
> @@ -827,6 +863,7 @@ HRESULT send_hello_message(IWSDiscoveryPublisherImpl *impl, LPCWSTR id, ULONGLON
>      WSD_APP_SEQUENCE sequence;
>      WCHAR message_id[64];
>      HRESULT ret = E_OUTOFMEMORY;
> +    LPWSTR buffer;
>
> sequence.InstanceId = instance_id;
>      sequence.MessageNumber = msg_num;
> @@ -866,6 +903,19 @@ HRESULT send_hello_message(IWSDiscoveryPublisherImpl *impl, LPCWSTR id, ULONGLON
>          if (FAILED(ret)) goto cleanup;
>      }
>
> +    /* <wsd:Types> */
> +    if (types_list != NULL)
> +    {
> +        buffer = WSDAllocateLinkedMemory(hello_element, WSD_MAX_TEXT_LENGTH * sizeof(WCHAR));
> +        if (buffer == NULL) goto cleanup;

So you'll return S_OK in this case which is probably not right.
> +
> +        ret = build_types_list(buffer, WSD_MAX_TEXT_LENGTH * sizeof(WCHAR), types_list, discoveredNamespaces);
> +        if (FAILED(ret)) goto cleanup;
> +
> +        ret = add_child_element(impl->xmlContext, hello_element, discoveryNsUri, typesString, buffer, NULL);
> +        if (FAILED(ret)) goto cleanup;
> +    }
> +
>      /* Write any body elements */
>      if (any != NULL)
>      {

>

[Huw Davies]

On Fri, Apr 20, 2018 at 08:46:23AM +0100, Huw Davies wrote:
> On Fri, Apr 20, 2018 at 08:18:19AM +0100, Owen Rudge wrote:
> > Signed-off-by: Owen Rudge <orudge(a)codeweavers.com>
> > +static HRESULT build_types_list(LPWSTR buffer, size_t buffer_size, const WSD_NAME_LIST *list, struct list *namespaces)
> > +{
> > +    WCHAR format_string[] = { '%', 's', ':', '%', 's', ' ', 0 };
> > +    LPWSTR current_buf_pos = buffer;
> > +    size_t memory_needed = 0;
> > +    const WSD_NAME_LIST *cur = list;
> > +
> > +    ZeroMemory(buffer, buffer_size);
> > +
> > +    while (cur != NULL)
> > +    {
> > +        /* Calculate space needed, including NULL character, colon and potential trailing space */
> > +        memory_needed = sizeof(WCHAR) * (lstrlenW(cur->Element->LocalName) +
> > +            lstrlenW(cur->Element->Space->PreferredPrefix) + 3);
> > +
> > +        if (current_buf_pos + memory_needed > buffer + buffer_size)
> > +            return E_INVALIDARG;
> > +
> > +        current_buf_pos += wsprintfW(current_buf_pos, format_string, cur->Element->Space->PreferredPrefix,
> > +            cur->Element->LocalName);
> > +
> > +        /* Record the namespace in the discovered namespaces list */
> > +        if (!add_discovered_namespace(namespaces, cur->Element->Space))
> > +            return E_FAIL;
> > +
> > +        cur = cur->Next;
> > +    }
> > +
> > +    /* Remove the last trailing space */
> > +    current_buf_pos--;
> > +    *current_buf_pos = 0;
>
> Is it possible that list could be NULL so that the while loop is never
> executed?  If so, then the above line would touch buffer[-1].  If
> list is never NULL consider rewriting the loop as a do-while.

And actually, thinking about it a bit more, it would most likely be
cleaner to add the space at the beginning of the loop if (cur != list).

Huw.

[Owen Rudge]

Hi Huw,
> Is it possible that list could be NULL so that the while loop is never
> executed?  If so, then the above line would touch buffer[-1].  If
> list is never NULL consider rewriting the loop as a do-while.

No, the list is checked for NULL in the calling function. I can rewrite it as a do-while though for clarity.

Thanks,

Owen

[Huw Davies]

On Fri, Apr 20, 2018 at 08:16:55AM +0000, Owen Rudge wrote:
> Hi Huw,
> > Is it possible that list could be NULL so that the while loop is never
> > executed?  If so, then the above line would touch buffer[-1].  If
> > list is never NULL consider rewriting the loop as a do-while.
>
> No, the list is checked for NULL in the calling function. I can rewrite it as a do-while though for clarity.

While you're at it, is that ZeroMemory() call really necessary?

Huw.

[Owen Rudge]

> While you're at it, is that ZeroMemory() call really necessary?

It probably isn't required any more, I'll check and remove it.

Thanks,

Owen