Re: [FFmpeg-devel] [PATCH v1 2/2] libavformat/file: initilize the fd to -1 instead of 0(valid fd) in case unexpected file close
On Sat, Jul 13, 2019 at 06:36:28AM +0800, Limin Wang wrote: > On Fri, Jul 12, 2019 at 10:44:48PM +0200, Michael Niedermayer wrote: > > On Tue, Jun 18, 2019 at 06:45:13PM +0800, lance.lmw...@gmail.com wrote: > > > From: Limin Wang > > > > > > Signed-off-by: Limin Wang > > > --- > > > libavformat/file.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/libavformat/file.c b/libavformat/file.c > > > index 08c7f8e6dd..40ae9ad2a8 100644 > > > --- a/libavformat/file.c > > > +++ b/libavformat/file.c > > > @@ -274,6 +274,7 @@ static int file_open_dir(URLContext *h) > > > #if HAVE_LSTAT > > > FileContext *c = h->priv_data; > > > > > > +c->fd = -1; > > > c->dir = opendir(h->filename); > > > if (!c->dir) > > > return AVERROR(errno); > > > > Is it more robust to set fd=-1 after successfully opening the directory ? > > > > considering that this could be already opened as a file ... > > (such combinations seem to be what the patch is about IIUC) > > and that on failure we could prefer to leave the state close to the > > original before the call > I'm using open the directory operation without any file combiations, if > the context need to support both dir and file combined, then I need do more > testing for the condition. It doesnt need no, but people do mix it as it seems. So the behvior in that case should be "usefull" like erroring our or failing an assert while in this case i think it would just leak the open file and continue that may be harder to debug for the user thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The educated differ from the uneducated as much as the living from the dead. -- Aristotle signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v1 2/2] libavformat/file: initilize the fd to -1 instead of 0(valid fd) in case unexpected file close
On Fri, Jul 12, 2019 at 10:44:48PM +0200, Michael Niedermayer wrote: > On Tue, Jun 18, 2019 at 06:45:13PM +0800, lance.lmw...@gmail.com wrote: > > From: Limin Wang > > > > Signed-off-by: Limin Wang > > --- > > libavformat/file.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/libavformat/file.c b/libavformat/file.c > > index 08c7f8e6dd..40ae9ad2a8 100644 > > --- a/libavformat/file.c > > +++ b/libavformat/file.c > > @@ -274,6 +274,7 @@ static int file_open_dir(URLContext *h) > > #if HAVE_LSTAT > > FileContext *c = h->priv_data; > > > > +c->fd = -1; > > c->dir = opendir(h->filename); > > if (!c->dir) > > return AVERROR(errno); > > Is it more robust to set fd=-1 after successfully opening the directory ? > > considering that this could be already opened as a file ... > (such combinations seem to be what the patch is about IIUC) > and that on failure we could prefer to leave the state close to the > original before the call I'm using open the directory operation without any file combiations, if the context need to support both dir and file combined, then I need do more testing for the condition. > > thanks > > [...[ > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > I have never wished to cater to the crowd; for what I know they do not > approve, and what they approve I do not know. -- Epicurus > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v1 2/2] libavformat/file: initilize the fd to -1 instead of 0(valid fd) in case unexpected file close
On Tue, Jun 18, 2019 at 06:45:13PM +0800, lance.lmw...@gmail.com wrote: > From: Limin Wang > > Signed-off-by: Limin Wang > --- > libavformat/file.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/libavformat/file.c b/libavformat/file.c > index 08c7f8e6dd..40ae9ad2a8 100644 > --- a/libavformat/file.c > +++ b/libavformat/file.c > @@ -274,6 +274,7 @@ static int file_open_dir(URLContext *h) > #if HAVE_LSTAT > FileContext *c = h->priv_data; > > +c->fd = -1; > c->dir = opendir(h->filename); > if (!c->dir) > return AVERROR(errno); Is it more robust to set fd=-1 after successfully opening the directory ? considering that this could be already opened as a file ... (such combinations seem to be what the patch is about IIUC) and that on failure we could prefer to leave the state close to the original before the call thanks [...[ -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I have never wished to cater to the crowd; for what I know they do not approve, and what they approve I do not know. -- Epicurus signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v1 2/2] libavformat/file: initilize the fd to -1 instead of 0(valid fd) in case unexpected file close
On Thu, Jul 11, 2019 at 11:16:49AM +0800, myp...@gmail.com wrote:
> On Wed, Jul 10, 2019 at 11:11 PM Limin Wang wrote:
> >
> >
> > ping? I have developed code to use avio_open_dir function, after using
> > avio_close_dir
> > to release the resource, my ffmepg command will lost interact for the fd
> > 0 is closed by avio_close_dir.
> >
> >
> > On Tue, Jun 18, 2019 at 06:45:13PM +0800, lance.lmw...@gmail.com wrote:
> > > From: Limin Wang
> > >
> > > Signed-off-by: Limin Wang
> > > ---
> > > libavformat/file.c | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > diff --git a/libavformat/file.c b/libavformat/file.c
> > > index 08c7f8e6dd.. 40ae9ad2a8 100644
> > > --- a/libavformat/file.c
> > > +++ b/libavformat/file.c
> > > @@ -274,6 +274,7 @@ static int file_open_dir(URLContext *h)
> > > #if HAVE_LSTAT
> > > FileContext *c = h->priv_data;
> > >
> > > +c->fd = -1;
> I don't konw why need to change the fd , suppose we have a calling
> sequence like file_open ahead of file_open_dir, can we leak a fd
> resource ?
How to reproduce the issue:
1. add one debug message like below:
diff --git a/libavformat/file.c b/libavformat/file.c
index 08c7f8e6dd..f06f1aa506 100644
--- a/libavformat/file.c
+++ b/libavformat/file.c
@@ -266,6 +266,8 @@ static int64_t file_seek(URLContext *h, int64_t pos,
int whence)
static int file_close(URLContext *h)
{
FileContext *c = h->priv_data;
+
+av_log(h, AV_LOG_INFO, "fd: %d\n", c->fd);
return close(c->fd);
}
2. build the examples with make examples
3. run below example tool:
./doc/examples/avio_dir_cmd list ./doc/examples
78056 transcoding.o501(20) 644
156281847900 156281894900 156281847900
18744308 muxing501(20) 755
156281894800 156281894800 156281894800
[file @ 0x7f8e50406700] fd: 0
You can see valid fd 0 is closed unexpected, if you run in ffmpeg with
avio_dir_close option, you'll lose interaction cli control for the
close.
> > > c->dir = opendir(h->filename);
> > > if (!c->dir)
> > > return AVERROR(errno);
> > > --
> > > 2.21.0
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v1 2/2] libavformat/file: initilize the fd to -1 instead of 0(valid fd) in case unexpected file close
On Wed, Jul 10, 2019 at 11:11 PM Limin Wang wrote: > > > ping? I have developed code to use avio_open_dir function, after using > avio_close_dir > to release the resource, my ffmepg command will lost interact for the fd > 0 is closed by avio_close_dir. > > > On Tue, Jun 18, 2019 at 06:45:13PM +0800, lance.lmw...@gmail.com wrote: > > From: Limin Wang > > > > Signed-off-by: Limin Wang > > --- > > libavformat/file.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/libavformat/file.c b/libavformat/file.c > > index 08c7f8e6dd.. 40ae9ad2a8 100644 > > --- a/libavformat/file.c > > +++ b/libavformat/file.c > > @@ -274,6 +274,7 @@ static int file_open_dir(URLContext *h) > > #if HAVE_LSTAT > > FileContext *c = h->priv_data; > > > > +c->fd = -1; I don't konw why need to change the fd , suppose we have a calling sequence like file_open ahead of file_open_dir, can we leak a fd resource ? > > c->dir = opendir(h->filename); > > if (!c->dir) > > return AVERROR(errno); > > -- > > 2.21.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v1 2/2] libavformat/file: initilize the fd to -1 instead of 0(valid fd) in case unexpected file close
ping? I have developed code to use avio_open_dir function, after using avio_close_dir to release the resource, my ffmepg command will lost interact for the fd 0 is closed by avio_close_dir. On Tue, Jun 18, 2019 at 06:45:13PM +0800, lance.lmw...@gmail.com wrote: > From: Limin Wang > > Signed-off-by: Limin Wang > --- > libavformat/file.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/libavformat/file.c b/libavformat/file.c > index 08c7f8e6dd..40ae9ad2a8 100644 > --- a/libavformat/file.c > +++ b/libavformat/file.c > @@ -274,6 +274,7 @@ static int file_open_dir(URLContext *h) > #if HAVE_LSTAT > FileContext *c = h->priv_data; > > +c->fd = -1; > c->dir = opendir(h->filename); > if (!c->dir) > return AVERROR(errno); > -- > 2.21.0 > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v1 2/2] libavformat/file: initilize the fd to -1 instead of 0(valid fd) in case unexpected file close
From: Limin Wang Signed-off-by: Limin Wang --- libavformat/file.c | 1 + 1 file changed, 1 insertion(+) diff --git a/libavformat/file.c b/libavformat/file.c index 08c7f8e6dd..40ae9ad2a8 100644 --- a/libavformat/file.c +++ b/libavformat/file.c @@ -274,6 +274,7 @@ static int file_open_dir(URLContext *h) #if HAVE_LSTAT FileContext *c = h->priv_data; +c->fd = -1; c->dir = opendir(h->filename); if (!c->dir) return AVERROR(errno); -- 2.21.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
