Re: [FFmpeg-devel] [PATCH] Adding mkdir option for img2enc (3rd attempt)

[Marton Balint]

On Wed, 17 Jan 2018, Dr. Alan Barclay wrote:


Hi,

Attached in a further patch - adding the error checks.

Derek - I don't think there is a functional downside to this 'mkdir' option 
being a default behaviour, but it would introduce a minor performance penalty 
(which users maybe don't want).


I'd rather keep the mkdir functionality disabled by default, from a 
security standpoint I don't really like the idea of an ordinary muxer 
creating dirs, replacing parts of the path as default behaviour...


Thanks,
Marton



All comments and help appreciated.

Thanks and Regards,
Alan.


On 27/12/2017 16:01, Derek Buitenhuis wrote:

Hi,

On 12/27/2017 12:27 PM, Dr Alan Barclay wrote:

Resending the two (git format-patch) patches, without the top lines
removed (which I thought I needed to do as some patch emails didn't seem
to have them).

All comments and help appreciated.


[...]


Subject: [PATCH 1/2] Move mkdir_p (renamed ff_mkdir_p) from hlsenc.c to
  utils.c.

---
  libavformat/hlsenc.c   | 35 +--
  libavformat/internal.h |  7 +++
  libavformat/utils.c| 33 +
  3 files changed, 41 insertions(+), 34 deletions(-)


On a technical level, this patch looks OK.


Subject: [PATCH 2/2] Adding mkdir option for img2enc.

---
  libavformat/img2enc.c | 8 
  1 file changed, 8 insertions(+)


I'm not sure how others feel about the premise (mkdir in img2enc).
I personally don't mind - though, maybe it should be default instead
of an option? (Maybe a bad idea.)


+if (img->use_mkdir) {
+char *temp_filename = av_strdup(filename);
+const char *temp_path = av_dirname(temp_filename);
+ff_mkdir_p(temp_path);
+av_free(temp_filename);
+}


This lacks error checks for av_strdup and ff_mkdir_p.

- Derek

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel




--
Dr. Alan Barclay
Electric Scribe Ltd.
118 Stanley Street
Aberdeen AB10 6UQ, U.K.
+44 1224 591779 office
+44 7803 606485 mobile
a...@escribe.co.uk


___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] Adding mkdir option for img2enc (3rd attempt)

[Carl Eugen Hoyos]

2018-01-17 11:56 GMT+01:00 Dr. Alan Barclay :

> Attached in a further patch - adding the error checks.

Please merge this patch into your previous patch.

And please avoid top-posting here, Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] Adding mkdir option for img2enc (3rd attempt)

[Dr. Alan Barclay]

Hi,

Attached in a further patch - adding the error checks.

Derek - I don't think there is a functional downside to this 'mkdir' 
option being a default behaviour, but it would introduce a minor 
performance penalty (which users maybe don't want).


All comments and help appreciated.

Thanks and Regards,
Alan.


On 27/12/2017 16:01, Derek Buitenhuis wrote:

Hi,

On 12/27/2017 12:27 PM, Dr Alan Barclay wrote:

Resending the two (git format-patch) patches, without the top lines
removed (which I thought I needed to do as some patch emails didn't seem
to have them).

All comments and help appreciated.


[...]


Subject: [PATCH 1/2] Move mkdir_p (renamed ff_mkdir_p) from hlsenc.c to
  utils.c.

---
  libavformat/hlsenc.c   | 35 +--
  libavformat/internal.h |  7 +++
  libavformat/utils.c| 33 +
  3 files changed, 41 insertions(+), 34 deletions(-)


On a technical level, this patch looks OK.


Subject: [PATCH 2/2] Adding mkdir option for img2enc.

---
  libavformat/img2enc.c | 8 
  1 file changed, 8 insertions(+)


I'm not sure how others feel about the premise (mkdir in img2enc).
I personally don't mind - though, maybe it should be default instead
of an option? (Maybe a bad idea.)


+if (img->use_mkdir) {
+char *temp_filename = av_strdup(filename);
+const char *temp_path = av_dirname(temp_filename);
+ff_mkdir_p(temp_path);
+av_free(temp_filename);
+}


This lacks error checks for av_strdup and ff_mkdir_p.

- Derek

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel




--
Dr. Alan Barclay
Electric Scribe Ltd.
118 Stanley Street
Aberdeen AB10 6UQ, U.K.
+44 1224 591779 office
+44 7803 606485 mobile
a...@escribe.co.uk
From a85be3fbf11f40567370c58f6c7e72d7edbd3109 Mon Sep 17 00:00:00 2001
From: "Dr. Alan Barclay" 
Date: Wed, 17 Jan 2018 10:44:17 +
Subject: [PATCH 2/2] Added error checking for mkdir option.

---
 libavformat/img2enc.c | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/libavformat/img2enc.c b/libavformat/img2enc.c
index a8ee064396..9de2c8f356 100644
--- a/libavformat/img2enc.c
+++ b/libavformat/img2enc.c
@@ -116,9 +116,17 @@ static int write_packet(AVFormatContext *s, AVPacket *pkt)
 return AVERROR(EINVAL);
 }
 if (img->use_mkdir) {
+const char *temp_path;
 char *temp_filename = av_strdup(filename);
-const char *temp_path = av_dirname(temp_filename);
-ff_mkdir_p(temp_path);
+if (!temp_filename) {
+return AVERROR(ENOMEM);
+}
+temp_path = av_dirname(temp_filename);
+if (ff_mkdir_p(temp_path) == -1 && errno != EEXIST) {
+av_log(s, AV_LOG_ERROR, "Could not create directory %s\n", 
temp_path);
+av_free(temp_filename);
+return AVERROR(errno);
+}
 av_free(temp_filename);
 }
 for (i = 0; i < 4; i++) {
-- 
2.11.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel