Message ID | 834b1299-267c-2010-aa2e-a23f3a458820@escribe.co.uk |
---|---|
State | New |
Headers | show |
2018-01-17 11:56 GMT+01:00 Dr. Alan Barclay <alan@escribe.co.uk>:
> 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
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 > alan@escribe.co.uk >
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++) {