| Message ID | 20170311233353.4836-2-jamrial@gmail.com |
|---|---|
| State | Accepted |
| Commit | ff17c76e92cd9a9072a8771cad73c96cd620040b |
| Headers | show |
On Sat, Mar 11, 2017 at 08:33:53PM -0300, James Almer wrote: > Signed-off-by: James Almer <jamrial@gmail.com> > --- > tests/api/api-seek-test.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/tests/api/api-seek-test.c b/tests/api/api-seek-test.c > index 6ef3b91933..503968fa13 100644 > --- a/tests/api/api-seek-test.c > +++ b/tests/api/api-seek-test.c > @@ -40,8 +40,8 @@ static int add_crc_to_array(uint32_t crc, int64_t pts) > if (size_of_array == 0) > size_of_array = 10; > size_of_array *= 2; > - crc_array = av_realloc(crc_array, size_of_array * sizeof(uint32_t)); > - pts_array = av_realloc(pts_array, size_of_array * sizeof(int64_t)); > + crc_array = av_realloc_f(crc_array, size_of_array, sizeof(uint32_t)); > + pts_array = av_realloc_f(pts_array, size_of_array, sizeof(int64_t)); > if ((crc_array == NULL) || (pts_array == NULL)) { > av_log(NULL, AV_LOG_ERROR, "Can't allocate array to store crcs\n"); > return AVERROR(ENOMEM); thats not enough, one of the reallocs can fail and one can succeed the succeeded one would leak unless iam missing something [...]
On 3/12/2017 10:12 AM, Michael Niedermayer wrote: > On Sat, Mar 11, 2017 at 08:33:53PM -0300, James Almer wrote: >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> tests/api/api-seek-test.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/tests/api/api-seek-test.c b/tests/api/api-seek-test.c >> index 6ef3b91933..503968fa13 100644 >> --- a/tests/api/api-seek-test.c >> +++ b/tests/api/api-seek-test.c >> @@ -40,8 +40,8 @@ static int add_crc_to_array(uint32_t crc, int64_t pts) >> if (size_of_array == 0) >> size_of_array = 10; >> size_of_array *= 2; >> - crc_array = av_realloc(crc_array, size_of_array * sizeof(uint32_t)); >> - pts_array = av_realloc(pts_array, size_of_array * sizeof(int64_t)); >> + crc_array = av_realloc_f(crc_array, size_of_array, sizeof(uint32_t)); >> + pts_array = av_realloc_f(pts_array, size_of_array, sizeof(int64_t)); >> if ((crc_array == NULL) || (pts_array == NULL)) { >> av_log(NULL, AV_LOG_ERROR, "Can't allocate array to store crcs\n"); >> return AVERROR(ENOMEM); > > thats not enough, one of the reallocs can fail and one can succeed > the succeeded one would leak unless iam missing something The program calls av_freep for both arrays as part of the cleaning process at the end but apparently only on success and not on failure, so you're right that it's not enough. The entire program leaks everything on any kind failure, not just failed allocation. I'll see about sending a patch to properly uninit everything on failure.
On 3/12/2017 10:12 AM, Michael Niedermayer wrote: > On Sat, Mar 11, 2017 at 08:33:53PM -0300, James Almer wrote: >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> tests/api/api-seek-test.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/tests/api/api-seek-test.c b/tests/api/api-seek-test.c >> index 6ef3b91933..503968fa13 100644 >> --- a/tests/api/api-seek-test.c >> +++ b/tests/api/api-seek-test.c >> @@ -40,8 +40,8 @@ static int add_crc_to_array(uint32_t crc, int64_t pts) >> if (size_of_array == 0) >> size_of_array = 10; >> size_of_array *= 2; >> - crc_array = av_realloc(crc_array, size_of_array * sizeof(uint32_t)); >> - pts_array = av_realloc(pts_array, size_of_array * sizeof(int64_t)); >> + crc_array = av_realloc_f(crc_array, size_of_array, sizeof(uint32_t)); >> + pts_array = av_realloc_f(pts_array, size_of_array, sizeof(int64_t)); >> if ((crc_array == NULL) || (pts_array == NULL)) { >> av_log(NULL, AV_LOG_ERROR, "Can't allocate array to store crcs\n"); >> return AVERROR(ENOMEM); > > thats not enough, one of the reallocs can fail and one can succeed > the succeeded one would leak unless iam missing something Pushed after addressing the cleanup on failure issues. Thanks.
diff --git a/tests/api/api-seek-test.c b/tests/api/api-seek-test.c index 6ef3b91933..503968fa13 100644 --- a/tests/api/api-seek-test.c +++ b/tests/api/api-seek-test.c @@ -40,8 +40,8 @@ static int add_crc_to_array(uint32_t crc, int64_t pts) if (size_of_array == 0) size_of_array = 10; size_of_array *= 2; - crc_array = av_realloc(crc_array, size_of_array * sizeof(uint32_t)); - pts_array = av_realloc(pts_array, size_of_array * sizeof(int64_t)); + crc_array = av_realloc_f(crc_array, size_of_array, sizeof(uint32_t)); + pts_array = av_realloc_f(pts_array, size_of_array, sizeof(int64_t)); if ((crc_array == NULL) || (pts_array == NULL)) { av_log(NULL, AV_LOG_ERROR, "Can't allocate array to store crcs\n"); return AVERROR(ENOMEM);
Signed-off-by: James Almer <jamrial@gmail.com> --- tests/api/api-seek-test.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)