[FFmpeg-devel] Patch: libssh on Windows

Reimar Döffinger Reimar.Doeffinger at gmx.de
Sat Dec 28 15:55:55 CET 2013


On Sat, Dec 28, 2013 at 03:48:58PM +0100, Lukasz M wrote:
> On 28 December 2013 15:41, Reimar Döffinger <Reimar.Doeffinger at gmx.de>wrote:
> 
> > On Sat, Dec 28, 2013 at 03:32:08PM +0100, Lukasz M wrote:
> > > On 28 December 2013 15:26, Reimar Döffinger <Reimar.Doeffinger at gmx.de
> > >wrote:
> > >
> > > > On Sat, Dec 28, 2013 at 11:58:29AM +0000, Carl Eugen Hoyos wrote:
> > > > > Matt Oliver <protogonoi <at> gmail.com> writes:
> > > > >
> > > > > > This is just a quick patch to allow for libssh.c to
> > > > > > compile on any Windows platform. The Windows headers
> > > > > > dont define some of the standard read/write
> > > > > > permission flags (S_IRUSR etc.).
> > > > >
> > > > > I believe this belongs into libavformat/os_support.h
> > > >
> > > > They are already there (at the very least some).
> > > > Probably just a missing include.
> > >
> > >
> > > There are S_IRUSR S_IWUSR
> > > S_IRGRP, S_IWGRP, S_IROTH are missing
> > >
> > > If it is ok I would add following to os_support,h and include it in
> > libssh.c
> > >
> > > # define S_IRGRP        (S_IRUSR >> 3)  /* Read by group.  */
> > > # define S_IWGRP        (S_IWUSR >> 3)  /* Write by group.  */
> > > # define S_IROTH        (S_IRGRP >> 3)  /* Read by others.  */
> >
> > I am not sure this is correct, particularly on Windows.
> > I suspect that the correct thing would be to define them to 0 if they
> > are not defined (which is probably why they are currently not defined,
> > since they are a bit tricky to define in a way that makes sense on OSs
> > that do not have the UNIX ugo permission scheme).
> > Alternatively putting the code using them under ifdef would be another
> > option. Haven't checked which might make more sense.
> 
> 
> I know what you mean, but libssh.c code use these perms to define
> permissions of newly created file on remote host (local also is possible
> when you use localhost as host address), and remote host can be non Windows.
> It is not a problem to define them to zero, but file will be accessible by
> the user only.
> 
> Other options would be to define S_IRGRP, S_IWGRP, S_IROTH to non-zero
> inside libssh,c only, or just use hardcoded value as sftp_open param.
> Last one may seem to be ugly, but almost sure I saw such a thing inside
> ffmpeg.
> 
> Anyway, either way is fine with me so I leave decision for others.

As far as I can tell, libssh packs this value directly into the network
message.
IMHO this means that using S_IRUSR etc. in our libssh code is just plain
wrong (though the many FIXMEs in the code would indicate to me that
they've never really properly though about the issue).
We need to use the values that the ssh protocol specifies (note: I do
not know if such a specification exists).
I suspect we should be using a hard-coded 0664 in the sftp_open call instead of the
ORing we have currently.


More information about the ffmpeg-devel mailing list