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

[Owen Rudge]

Signed-off-by: Owen Rudge 
---
   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
>
> 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 
> > +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