[FFmpeg-devel] Patch: libssh on Windows

[Lukasz M]

On 28 December 2013 15:55, Reimar Döffinger <Reimar.Doeffinger at gmx.de>wrote:

> 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.


As I remember libssh has no own flags. They use S_I* in their example:
http://api.libssh.org/master/libssh_tutor_sftp.html
Using 0664 seems to be OK.