Bug 46049

(In reply to comment #0)
> * if it's meaningful, make it happen, perhaps by implementing
>   a stub _write function on WinCE which only allows the fd to be 1 or 2,
>   and uses Standard C fwrite() to write to stdout or stderr respectively

Indeed, using fwrite() to stdout/stderr might be the way to go on mainstream Windows too, if it's not possible to start a process with a fd > 2 pointing into a pipe or file opened by the parent process. In a Unix shell you'd do that like this, for instance:

    dbus-daemon --session --print-address=7 7> address.tmp

(where the choice of 6 is arbitrary - any decimal number greater than 2 would work.)

*** Bug 42440 has been marked as a duplicate of this bug. ***

(In reply to comment #1)
> (In reply to comment #0)
> > * if it's meaningful, make it happen, perhaps by implementing
> >   a stub _write function on WinCE which only allows the fd to be 1 or 2,
> >   and uses Standard C fwrite() to write to stdout or stderr respectively
> 
> Indeed, using fwrite() to stdout/stderr might be the way to go on mainstream
> Windows too, if it's not possible to start a process with a fd > 2 pointing
> into a pipe or file opened by the parent process. In a Unix shell you'd do that
> like this, for instance:
> 
>     dbus-daemon --session --print-address=7 7> address.tmp
> 
> (where the choice of 6 is arbitrary - any decimal number greater than 2 would
> work.)

dbus-daemon --session --print-address > address.tmp

produces the following output 

C:\kde\trunk\git\dbus-src-git> dbus-daemon --session --print-address
tcp:host=localhost,port=30954,family=ipv4,guid=2eb29d82f2fb52d79c794cf44f4789c5

dbus-daemon --session --print-address=7 7> address.tmp

results in a debug assertation in the visual c runtime crt/src/write.c:68 Expression: (_osfile(fh) & FOPEN)

(In reply to comment #3)
> (In reply to comment #1)
> > (In reply to comment #0)
> > > * if it's meaningful, make it happen, perhaps by implementing
> > >   a stub _write function on WinCE which only allows the fd to be 1 or 2,
> > >   and uses Standard C fwrite() to write to stdout or stderr respectively
> > 
> > Indeed, using fwrite() to stdout/stderr might be the way to go on mainstream
> > Windows too, if it's not possible to start a process with a fd > 2 pointing
> > into a pipe or file opened by the parent process. In a Unix shell you'd do that
> > like this, for instance:
> > 
> >     dbus-daemon --session --print-address=7 7> address.tmp
> > 
> > (where the choice of 6 is arbitrary - any decimal number greater than 2 would
> > work.)
> 
> dbus-daemon --session --print-address > address.tmp
> 
> produces the following output 
> 
> C:\kde\trunk\git\dbus-src-git> dbus-daemon --session --print-address
> tcp:host=localhost,port=30954,family=ipv4,guid=2eb29d82f2fb52d79c794cf44f4789c5
> 
> dbus-daemon --session --print-address=7 7> address.tmp
> 
> results in a debug assertation in the visual c runtime crt/src/write.c:68
> Expression: (_osfile(fh) & FOPEN)

running under a debugger shows, that this happens in the following function at the location marked with !!

int
_dbus_pipe_write (DBusPipe         *pipe,
                  const DBusString *buffer,
                  int               start,
                  int               len,
                  DBusError        *error)
{
  const char *buffer_c = _dbus_string_get_const_data (buffer);
  int written;

!!  written = _write (pipe->fd, buffer_c + start, len);


I assume, that checking the state of the fd and making sure it is open for write may fix this issue.

(In reply to comment #3)
> dbus-daemon --session --print-address > address.tmp
> 
> produces the following output 
> 
> C:\kde\trunk\git\dbus-src-git> dbus-daemon --session --print-address
> tcp:host=localhost,port=30954,family=ipv4,guid=2eb29d82f2fb52d79c794cf44f4789c5

This looks correct. Is that with or without my patch? Under Wine, without the patch I get something like "error writing to pipe: Success", and with the patch I get output similar to this.

(In reply to comment #3)
> > In a Unix shell you'd do that
> > like this, for instance:
> > 
> >     dbus-daemon --session --print-address=7 7> address.tmp

The meaning of that shell syntax is: start "dbus-daemon --session --print-address=7", with address.tmp already open for writing as file descriptor number 7 (the shell opens the file, and dbus-daemon inherits the file descriptor. Is this something that even makes sense in Windows?

If not, the thing to do would be to only allow writing to file descriptor 1 (which is fileno (stdout)) or 2 (which is fileno (stderr)).

(In reply to comment #5)
> (In reply to comment #3)
> > dbus-daemon --session --print-address > address.tmp
> > 
> > produces the following output 
> > 
> > C:\kde\trunk\git\dbus-src-git> dbus-daemon --session --print-address
> > tcp:host=localhost,port=30954,family=ipv4,guid=2eb29d82f2fb52d79c794cf44f4789c5
> 
> This looks correct. Is that with or without my patch? Under Wine, without the
> patch I get something like "error writing to pipe: Success", and with the patch
> I get output similar to this.
> 
> (In reply to comment #3)
> > > In a Unix shell you'd do that
> > > like this, for instance:
> > > 
> > >     dbus-daemon --session --print-address=7 7> address.tmp
> 
> The meaning of that shell syntax is: start "dbus-daemon --session
> --print-address=7", with address.tmp already open for writing as file
> descriptor number 7 (the shell opens the file, and dbus-daemon inherits the
> file descriptor. 

On http://www.microsoft.com/resources/documentation/windows/xp/all/proddocs/en-us/redirection.mspx?mfr=true if found a related hint 

UNDEFINED 3-9 These handles are defined individually by the application and are specific to each tool.

The comment is an additional indication for my assumption in comment #4. I will try to check if opening the fd in the application works. 

> Is this something that even makes sense in Windows?

This will be useful when there are more output channels required for example when using 

dbus-daemon --session --print-address=7  --print-pid=6  7> address.tmp 6> pid.tmp 2> error.tmp 1>stdout.tmp 

This may also be used in dbus-launch to fetch the output channels from the daemon. 
 
> If not, the thing to do would be to only allow writing to file descriptor 1
> (which is fileno (stdout)) or 2 (which is fileno (stderr)).

This works out of box at least on Windows 7. 

>This commit goes back to the C runtime API. It might cause WinCE support
>to regress, but at least dbus-daemon.exe --print-address works again.

What about to move the duplicate the recent dbus-pipe-win.c implementation to a file named dbus/dbus-pipe-wince.c. This would not break recent wince implementation and make the os dependent parts more visible in this area.

(In reply to comment #6)
> On
> http://www.microsoft.com/resources/documentation/windows/xp/all/proddocs/en-us/redirection.mspx?mfr=true
> if found a related hint 
> 
> UNDEFINED 3-9 These handles are defined individually by the application and are
> specific to each tool.
> 
> The comment is an additional indication for my assumption in comment #4. I will
> try to check if opening the fd in the application works. 

At http://developer.gnome.org/glib/2.30/glib-Spawning-Processes.html there is stated: 

G_SPAWN_LEAVE_DESCRIPTORS_OPEN
	the parent's open file descriptors will be inherited by the child; otherwise all descriptors except stdin/stdout/stderr will be closed before calling exec() in the child. 

As glib has been ported to windows the question is if this behavior has been ported too.

(In reply to comment #7)
> (In reply to comment #6)
> > On
> > http://www.microsoft.com/resources/documentation/windows/xp/all/proddocs/en-us/redirection.mspx?mfr=true
> > if found a related hint 
> > 
> > UNDEFINED 3-9 These handles are defined individually by the application and are
> > specific to each tool.
> > 
> > The comment is an additional indication for my assumption in comment #4. I will
> > try to check if opening the fd in the application works. 
> 
> At http://developer.gnome.org/glib/2.30/glib-Spawning-Processes.html there is
> stated: 
> 
> G_SPAWN_LEAVE_DESCRIPTORS_OPEN
>     the parent's open file descriptors will be inherited by the child;
> otherwise all descriptors except stdin/stdout/stderr will be closed before
> calling exec() in the child. 
> 
> As glib has been ported to windows the question is if this behavior has been
> ported too.

I just found another thread which relates to this topic http://mail.gnome.org/archives/gtk-list/2006-January/msg00156.html

(In reply to comment #8)
> > As glib has been ported to windows the question is if this behavior has been
> > ported too.

An example with pipe inheritance using the native win32 api is documented here http://msdn.microsoft.com/en-us/library/ms682499%28VS.85%29.aspx. [1]
 
> I just found another thread which relates to this topic
> http://mail.gnome.org/archives/gtk-list/2006-January/msg00156.html

In the opposite to the mentioned gtk thread we cannot change the implementation of the parent code.  

We need to get informations about how the mapping between handles and file descriptors is implemented in the parent (cmd.exe for example) and in the client.

Mapping a windows handle to a file descriptor could be done with _open_osfhandle http://msdn.microsoft.com/en-us/library/bdts1c9x%28v=vs.71%29.aspx

The opposite could be done with _get_osfhandle http://msdn.microsoft.com/en-us/library/ks2530z6.aspx

The example mentioned in [1] shows how standard file handles are transfered to the client by using members of the STARTINFO struct: 

   siStartInfo.hStdError = g_hChildStd_OUT_Wr;
   siStartInfo.hStdOutput = g_hChildStd_OUT_Wr;
   siStartInfo.hStdInput = g_hChildStd_IN_Rd;

but how are additional handles transfered ?

(In reply to comment #6)
> This will be useful when there are more output channels required for example
> when using 
> 
> dbus-daemon --session --print-address=7  --print-pid=6  7> address.tmp 6>
> pid.tmp 2> error.tmp 1>stdout.tmp

Yes, if this is something that can work, then it'd be useful to have (and this is how dbus-launch works on Unix) - but it's not clear to me whether it can work on Windows.

The argv that dbus-daemon would get if you do that in a Unix shell would just be:

    { "dbus-daemon", "--session", "--print=address=7", "--print-pid=6", NULL }

The bits with > are interpreted by the shell, and don't go to the child process at all.
 
(In reply to comment #6)
> The comment is an additional indication for my assumption in comment #4. I will
> try to check if opening the fd in the application works. 

The --print-address and --print-pid options are designed to be used with Unix fd semantics, in which the parent process (typically the shell - your 
equivalent would be cmd.exe) opens the file descriptors, and dbus-daemon inherits them. dbus-daemon opening a file isn't anywhere near as useful: it's more useful for it to write into a pipe opened by the parent process.

Part of the difference is fundamental to the OSs, I think: in Unix, file descriptors are a real thing in the operating system (the kernel communicates in terms of file descriptors), whereas in Windows, the real thing in the operating system is a HANDLE, and file descriptors are implemented in the C library in terms of HANDLEs. So I'm not sure that this can be implemented nicely in Windows at all.

> What about to move the duplicate the recent dbus-pipe-win.c implementation to a
> file named dbus/dbus-pipe-wince.c. This would not break recent wince
> implementation and make the os dependent parts more visible in this area.

I'm pretty sure the current implementation can't work either on Windows or WinCE: it's taking a file descriptor (small integer) and behaving as if it was a HANDLE (pointer). You wouldn't do this:

    WriteFile ((void *) 2, buffer, len, NULL, NULL);

and expect a successful write to stderr, but that's basically what dbus-pipe-win is doing.

Having separate Windows and WinCE implementations (with the Windows implementation using _write(), and the WinCE implementation limited to fds 1 and 2 and using fprintf() on the equivalent stdio FILE *) might still make sense; or we could just make the Windows implementation use stdio, and abandon trying to write to fds > 2.

I'm tempted to just say "on Windows, a numeric argument to --print-address and --print-pid can only be 1 or 2, for stdout or stderr", and if you need another mechanism in future, we could add one that fits Windows better. For instance, since Windows has named pipes, we could say that non-numeric arguments to --print-address or --print-pid on Windows are the names of named pipes, and then you could do this?

    parent process: open a named pipe with a random name, let's
    say its name is "\\.\pipe\dbus-E1XFW3Nfkr"

    parent process: start dbus-daemon as:
        dbus-daemon.exe --print-address="\\.\pipe\dbus-E1XFW3Nfkr"

    parent process: block in a read from the named pipe

    dbus-daemon: write address to "\\.\pipe\dbus-E1XFW3Nfkr"

    parent process: pick up the address from the named pipe
    and use it for whatever it wanted to do

(In reply to comment #9)

> but how are additional handles transfered ?

The file dospawn.c from the vc98 runtime ftp://ftp.cs.ntust.edu.tw/yang/PC-SIMSCRIPT/C++/VC98/CRT/SRC/DOSPAWN.C  (which is in this area the same as the recent msvc 2010 runtime) contains code to transfer all open file descriptors to the child using the lpReserved2 member of the StartupInfo struct 

        *((UNALIGNED int *)(StartupInfo.lpReserved2)) = nh;

        posfile = (char *)(StartupInfo.lpReserved2 + sizeof(int));
        posfhnd = (UNALIGNED intptr_t *)(StartupInfo.lpReserved2 +
            sizeof(int) + (nh * sizeof(char)));

which are fetched by the child and reassigned to the related file pointer (see ftp://ftp.cs.ntust.edu.tw/yang/PC-SIMSCRIPT/C++/VC98/CRT/SRC for more informations). 

But why does the example using in comment#3 fails, when all available file descriptors are inheritated from the parent ? Don't know yet.

(In reply to comment #11)
> which are fetched by the child and reassigned to the related file pointer (see
> ftp://ftp.cs.ntust.edu.tw/yang/PC-SIMSCRIPT/C++/VC98/CRT/SRC for more
> informations). 
> 
The link should be ftp://ftp.cs.ntust.edu.tw/yang/PC-SIMSCRIPT/C++/VC98/CRT/SRC/ioinit.c

(In reply to comment #10)
> (In reply to comment #6)
> > This will be useful when there are more output channels required for example
> > when using 
> > 
> > dbus-daemon --session --print-address=7  --print-pid=6  7> address.tmp 6>
> > pid.tmp 2> error.tmp 1>stdout.tmp
> 
> Yes, if this is something that can work, then it'd be useful to have (and this
> is how dbus-launch works on Unix) - but it's not clear to me whether it can
> work on Windows.
> 
> The argv that dbus-daemon would get if you do that in a Unix shell would just
> be:
> 
>     { "dbus-daemon", "--session", "--print=address=7", "--print-pid=6", NULL }
> 
> The bits with > are interpreted by the shell, and don't go to the child process
> at all.

sure, i did understand that 

> (In reply to comment #6)
> > The comment is an additional indication for my assumption in comment #4. I will
> > try to check if opening the fd in the application works. 
> 
> The --print-address and --print-pid options are designed to be used with Unix
> fd semantics, in which the parent process (typically the shell - your 
> equivalent would be cmd.exe) opens the file descriptors, and dbus-daemon
> inherits them. dbus-daemon opening a file isn't anywhere near as useful: it's
> more useful for it to write into a pipe opened by the parent process.
> 
> Part of the difference is fundamental to the OSs, I think: in Unix, file
> descriptors are a real thing in the operating system (the kernel communicates
> in terms of file descriptors), whereas in Windows, the real thing in the
> operating system is a HANDLE, and file descriptors are implemented in the C
> library in terms of HANDLEs. So I'm not sure that this can be implemented
> nicely in Windows at all.

As described in comment #11 there seems to be an implementation on windows which inherits file (and pipe) descriptors

(In reply to comment #13)

> As described in comment #11 there seems to be an implementation on windows
> which inherits file (and pipe) descriptors

here is a working example - http://msdn.microsoft.com/en-us/library/edze9h7e%28v=vs.100%29.aspx - it requires to use spawn.. to create the child process.

This implies the question why dbus-daemon is not able to use file descripter 7 in comment #3, where it should be able. cmd.exe uses also the c runtime and running 

 dbus-daemon --session --print-address=7 7> address.tmp

empties the file address.tmp, so the shell seems to be able to open the file descriptor. The conclusion is that there must be a problem in dbus-daemon's startup code not registering the inheritated file descriptors somehow.

(In reply to comment #14)
>
>  dbus-daemon --session --print-address=7 7> address.tmp
> 
> empties the file address.tmp, so the shell seems to be able to open the file
> descriptor. The conclusion is that there must be a problem in dbus-daemon's
> startup code not registering the inheritated file descriptors somehow.

My last assumption failed because it is cmd.exe which fails to inherit additional file descriptors to the child (tested on windows 7), which limits the usable file descriptors on the command line to 1 and 2. 

Spawning dbus-daemon from dbus-launch should be able to use --print-address=<fd> with fd > 2. 


(In reply to comment #5)
> (In reply to comment #3)
> > dbus-daemon --session --print-address > address.tmp
> > 
> > produces the following output 
> > 
> > C:\kde\trunk\git\dbus-src-git> dbus-daemon --session --print-address
> > tcp:host=localhost,port=30954,family=ipv4,guid=2eb29d82f2fb52d79c794cf44f4789c5
> 
> This looks correct. Is that with or without my patch? 

The patch is required to restore print-address functionality on win32 except wince. 

On wince it has to be checked if the c runtime has similar fd/handle inheritation feature.

Comment on attachment 57041 [details] [review]
Make dbus-daemon.exe --print-address work under Windows

Review of attachment 57041 [details] [review]:
-----------------------------------------------------------------

looks good, ship it.

(In reply to comment #16)
> looks good, ship it.

Thanks, in git for 1.5.12. This is a regression on WinCE, where D-Bus will no longer compile, AFAICS (but I think that's an acceptable price to pay for it working properly on Win32).

I'll attach a patch which should make it compile (but still not work) on WinCE, which would get WinCE back to how things were before I opened this bug, but leave Win32 working better. I doubt we're going to be able to test that patch, though.

Marcus (or anyone else really), if you're still interested in D-Bus on WinCE, please help with this bug.

Created attachment 57763 [details] [review]
DBusPipe: stub out the Windows implementation for Windows CE

The Win32 implementation uses _write() and _close() from the C runtime,
and according to MSDN those aren't available on WinCE.

This should get the WinCE implementation to at least compile, although
dbus-daemon.exe --print-address and --print-pid won't work.

(In reply to comment #6)
> What about to move the duplicate the recent dbus-pipe-win.c implementation to a
> file named dbus/dbus-pipe-wince.c. This would not break recent wince
> implementation and make the os dependent parts more visible in this area.

I didn't think it was worth introducing a new file into the build system for the stub implementation in Attachment #57763 [details]. It might still be worth doing if someone gives us a real, working WinCE implementation, since that's likely to be more code.

Stubbing it out with #ifdef on wince is good enough, nobody cares about these functions on Windows CE.

There are no pipes in Windows CE (and no inheritance of handles at all), we wrote our own pipe-like communication driver for the gnupg tool set, but it's not needed for a basic dbus implementation on windows CE.

BTW, as a general note, I strongly recommend to avoid using libc handles and functions like write_ in any Windows program, and use the windows api instead for everything.  Mixing them is just asking for trouble in the long run.  For example, the Windows C library opens stdout in text mode by default (and you have to reopen it in binary mode if you are sending non-printable characters to it).

As for Windows CE, the latest version of Windows CE is very different from the older versions, and may have more complete feature-parity with Windows regular, but I haven't checked it out.  It's really not a big deal if the Windows CE port is bit-rotting.  Note that most work of the port was put into making the Windows port more faithful to the Windows API, which I recommend anyway (see above).

(In reply to comment #20)
> BTW, as a general note, I strongly recommend to avoid using libc handles and
> functions like write_ in any Windows program, and use the windows api instead
> for everything.

Sure, I only reinstated use of (libc) file descriptors for DBusPipe, which it turns out is only used for --print-address and --print-pipe, which specifically deal with a file descriptor that's already open. Everything where parts of D-Bus open a file themselves is still in terms of HANDLEs.

(In reply to comment #20)
> Stubbing it out with #ifdef on wince is good enough, nobody cares about these
> functions on Windows CE.

Thanks. At the moment dbus master (and hence 1.6.x, when it happens) probably won't compile for WinCE. If you or Ralf or someone reviews Attachment #57763 [details], I can apply it, which should hopefully mean it compiles under WinCE (dbus-daemon.exe --print-address still won't work there, but you've indicated that that's meaningless anyway).

If you could test git master + Attachment #57763 [details] on WinCE, that'd be even better.

The patch here needs review from someone who isn't its author (i.e. not me), and preferably testing in a Windows CE environment. Marcus confirmed in Comment #20 that it is appropriate, at least in principle.

Until the patch is reviewed (i.e. someone has commented here saying they have reviewed it and have no objections), our policy is not to apply it, and until it's applied, D-Bus probably won't compile in a Windows CE environment. (I don't know whether anyone will actually notice that bug - does anyone use D-Bus on WinCE any more?)

(In reply to comment #23)
> The patch here needs review from someone who isn't its author (i.e. not me),
> and preferably testing in a Windows CE environment. 

which requires a working cross compile tool chain. I filed a related bug https://bugs.freedesktop.org/show_bug.cgi?id=50818

> Marcus confirmed in Comment #20 that it is appropriate, at least in principle.
> 
> Until the patch is reviewed (i.e. someone has commented here saying they have
> reviewed it and have no objections), our policy is not to apply it, and until
> it's applied, D-Bus probably won't compile in a Windows CE environment. (I
> don't know whether anyone will actually notice that bug - does anyone use D-Bus
> on WinCE any more?)

KDAB initiated the wince port.

WONTFIX since it seems nobody cares about CE any more.