Re: [PATCH 1/5] inetcomm: Implement IMimeBody DeleteProp

[Nikolay Sivov]

On 06/30/2016 11:58 AM, Alistair Leslie-Hughes wrote:
> Signed-off-by: Alistair Leslie-Hughes
>
> dlls/inetcomm/mimeole.c       | 23 +++++++++++++++++--
>   dlls/inetcomm/tests/mimeole.c | 53 +++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 74 insertions(+), 2 deletions(-)
>
> diff --git a/dlls/inetcomm/mimeole.c b/dlls/inetcomm/mimeole.c
> index 766503d..cff95a1 100644
> --- a/dlls/inetcomm/mimeole.c
> +++ b/dlls/inetcomm/mimeole.c
> @@ -898,8 +898,27 @@ static HRESULT WINAPI MimeBody_DeleteProp(
>                                    LPCSTR pszName)
>   {
>       MimeBody *This = impl_from_IMimeBody(iface);
> - FIXME("(%p)->(%s) stub\n", This, debugstr_a(pszName));
> - return E_NOTIMPL;
> - header_t *cursor, *cursor2;
> - BOOL found = FALSE;

You don't need to initialize 'found'.
> -
> - TRACE("(%p)->(%s) stub\n", This, debugstr_a(pszName));

'stub' should be removed, unless it's still too stubby.
> -
> - LIST_FOR_EACH_ENTRY_SAFE(cursor, cursor2, &This->headers, header_t, entry)
> - {
> -
>    if(ISPIDSTR(pszName))
> -
>        found = STRTOPID(pszName) == cursor->prop->id;
> -
>    else
> -
>        found = !lstrcmpiA(pszName, cursor->prop->name);
> -
> -
>    if(found)
> -
>    {
> -
>         list_remove(&cursor->entry);
> -
>         HeapFree(GetProcessHeap(), 0, cursor);
> -
>         return S_OK;
> -
>    }
> - }
> -
> - return MIME_E_NOT_FOUND;
> }

The way you use it, there's no need for _SAFE() list iteration.
> static HRESULT WINAPI MimeBody_CopyProps(
> diff --git a/dlls/inetcomm/tests/mimeole.c b/dlls/inetcomm/tests/mimeole.c
> index 8063b86..46fd1d5 100644
> --- a/dlls/inetcomm/tests/mimeole.c
> +++ b/dlls/inetcomm/tests/mimeole.c
> @@ -604,6 +604,58 @@ static void test_BindToObject(void)
>       IMimeMessage_Release(msg);
>   }
>
> +static void test_BodyDeleteProp(void)
> +{
> - static const char topic[] = "wine topic";
> - HRESULT hr;
> - IMimeMessage *msg;
> - IMimeBody *body;
> - PROPVARIANT prop;
> -
> - hr = MimeOleCreateMessage(NULL, &msg);
> - ok(hr == S_OK, "ret %08x\n", hr);
> -
> - PropVariantInit(&prop);
> -
> - hr = IMimeMessage_BindToObject(msg, HBODY_ROOT, &IID_IMimeBody, (void**)&body);
> - ok(hr == S_OK, "ret %08x\n", hr);
> -
> - hr = IMimeBody_DeleteProp(body, "Subject");
> - ok(hr == MIME_E_NOT_FOUND, "ret %08x\n", hr);
> -
> - hr = IMimeBody_DeleteProp(body, PIDTOSTR(PID_HDR_SUBJECT));
> - ok(hr == MIME_E_NOT_FOUND, "ret %08x\n", hr);
> -
> - prop.vt = VT_LPSTR;
> - prop.u.pszVal = CoTaskMemAlloc(strlen(topic)+1);
> - strcpy(prop.u.pszVal, topic);
> - hr = IMimeBody_SetProp(body, "Subject", 0, &prop);
> - ok(hr == S_OK, "ret %08x\n", hr);
> - PropVariantClear(&prop);
> -
> - hr = IMimeBody_DeleteProp(body, "Subject");
> - ok(hr == S_OK, "ret %08x\n", hr);
> -
> - hr = IMimeBody_GetProp(body, "Subject", 0, &prop);
> - ok(hr == MIME_E_NOT_FOUND, "ret %08x\n", hr);
> -
> - prop.vt = VT_LPSTR;
> - prop.u.pszVal = CoTaskMemAlloc(strlen(topic)+1);
> - strcpy(prop.u.pszVal, topic);
> - hr = IMimeBody_SetProp(body, PIDTOSTR(PID_HDR_SUBJECT), 0, &prop);
> - ok(hr == S_OK, "ret %08x\n", hr);
> - PropVariantClear(&prop);
> -
> - hr = IMimeBody_DeleteProp(body, PIDTOSTR(PID_HDR_SUBJECT));
> - ok(hr == S_OK, "ret %08x\n", hr);
> -
> - hr = IMimeBody_GetProp(body, PIDTOSTR(PID_HDR_SUBJECT), 0, &prop);
> - ok(hr == MIME_E_NOT_FOUND, "ret %08x\n", hr);
> -
> - IMimeBody_Release(body);
> - IMimeMessage_Release(msg);
>
> +}
> - static void test_MimeOleGetPropertySchema(void)
> {
>     HRESULT hr;
>
> @@ -627,6 +679,7 @@ START_TEST(mimeole)
>       test_MessageGetPropInfo();
>       test_MessageOptions();
>       test_BindToObject();
> - test_BodyDeleteProp();
>  test_MimeOleGetPropertySchema();
>  OleUninitialize();
> }