Puzzling IPrincipal:UserId check in taskschd

[Francois Gouget]

So I'm a bit puzzled by this check in taskschd/task.c's 
write_principal():

    hr = IPrincipal_get_UserId(principal, &bstr);
    if (hr == S_OK && lstrlenW(bstr))
    {
        hr = write_text_value(stream, L"UserId", bstr);

What's puzzling to me is the lstrlenW(bstr) check:

* write_principal() has no such check for Id, GroupId, DisplayName and
  RunLevel.

  Maybe that's because they are guaranteed to be set and non empty but I 
  could not find any documentation saying so and they are not 
  implemented in Wine.

* write_registration_info() is very similar but checks bstr, not what is 
  in effect *bstr:

    hr = IRegistrationInfo_get_Source(reginfo, &bstr);
    if (hr == S_OK && bstr)
    {
        hr = write_text_value(stream, L"Source", bstr);

* We don't seem to have tests showing how this works.


I would expect all these property getters to work the same way.
So would something like that make more sense?

    hr = IPrincipal_get_UserId(principal, &bstr);
    if (hr == S_OK && bstr)
    {
        hr = write_text_value(stream, L"UserId", bstr);


Dmitry: Do you remember if you wrote the lstrlenW(bstr) check 
        because of some test you did?

-- 
Francois Gouget <fgouget(a)free.fr>              http://fgouget.free.fr/
                      Computers are like airconditioners
                They stop working properly if you open WINDOWS

[Dmitry Timoshkov]

Francois Gouget <fgouget(a)free.fr> wrote:
> I would expect all these property getters to work the same way.
> So would something like that make more sense?
>
> hr = IPrincipal_get_UserId(principal, &bstr);
>     if (hr == S_OK && bstr)
>     {
>         hr = write_text_value(stream, L"UserId", bstr);

I'd guess the check is supposed to avoid writing an empty UserId, and
apparently the check for bstr being NULL is not enough. I don't recall
why writing empty UserId is different from other property values.

-- 
Dmitry.