| Message ID | 1493138713-2319-4-git-send-email-brian.starkey@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Brian Starkey <brian.starkey@arm.com> writes: > igt_get_all_cairo_formats() allocates the format list on the heap, but > returns it in a const pointer. Change this so that callers can free the > array without a warning, and free the array in all callers. > > Signed-off-by: Brian Starkey <brian.starkey@arm.com> > --- > lib/igt_fb.c | 4 +++- > lib/igt_fb.h | 2 +- > tests/kms_atomic.c | 3 ++- > tests/kms_render.c | 4 +++- > 4 files changed, 9 insertions(+), 4 deletions(-) > > diff --git a/lib/igt_fb.c b/lib/igt_fb.c > index d2b7e9e36606..b958c970973b 100644 > --- a/lib/igt_fb.c > +++ b/lib/igt_fb.c > @@ -1277,8 +1277,10 @@ const char *igt_format_str(uint32_t drm_format) > * > * This functions returns an array of all the drm fourcc codes supported by > * cairo and this library. > + * > + * The array should be freed by the caller. > */ > -void igt_get_all_cairo_formats(const uint32_t **formats, int *format_count) > +void igt_get_all_cairo_formats(uint32_t **formats, int *format_count) > { > static uint32_t *drm_formats; > static int n_formats; > diff --git a/lib/igt_fb.h b/lib/igt_fb.h > index 4a680cefb16d..e124910367a3 100644 > --- a/lib/igt_fb.h > +++ b/lib/igt_fb.h > @@ -154,7 +154,7 @@ int igt_cairo_printf_line(cairo_t *cr, enum igt_text_align align, > uint32_t igt_bpp_depth_to_drm_format(int bpp, int depth); > uint32_t igt_drm_format_to_bpp(uint32_t drm_format); > const char *igt_format_str(uint32_t drm_format); > -void igt_get_all_cairo_formats(const uint32_t **formats, int *format_count); > +void igt_get_all_cairo_formats(uint32_t **formats, int *format_count); > > #endif /* __IGT_FB_H__ */ > > diff --git a/tests/kms_atomic.c b/tests/kms_atomic.c > index 6375fede7179..9c03f6e21ebb 100644 > --- a/tests/kms_atomic.c > +++ b/tests/kms_atomic.c > @@ -807,7 +807,7 @@ static void atomic_state_free(struct kms_atomic_state *state) > static uint32_t plane_get_igt_format(struct kms_atomic_plane_state *plane) > { > drmModePlanePtr plane_kms; > - const uint32_t *igt_formats; > + uint32_t *igt_formats; > uint32_t ret = 0; > int num_igt_formats; > int i; > @@ -827,6 +827,7 @@ static uint32_t plane_get_igt_format(struct kms_atomic_plane_state *plane) > } > } > > + free(igt_formats); > drmModeFreePlane(plane_kms); > return ret; > } > diff --git a/tests/kms_render.c b/tests/kms_render.c > index fd33dfb7cafe..bc2ffc750c67 100644 > --- a/tests/kms_render.c > +++ b/tests/kms_render.c > @@ -176,7 +176,7 @@ static void test_connector(const char *test_name, > struct kmstest_connector_config *cconf, > enum test_flags flags) > { > - const uint32_t *formats; > + uint32_t *formats; > int format_count; > int i; > > @@ -193,6 +193,8 @@ static void test_connector(const char *test_name, > cconf, &cconf->connector->modes[0], > formats[i], flags); > } > + > + free(formats); Hi, Assuming I'm not missing anything, I think if you free formats here, the static variable in igt_get_all_cairo_formats() will point to garbage and further calls to that function will return uninitialized memory.
On Tue, Apr 25, 2017 at 02:43:13PM -0300, Gabriel Krisman Bertazi wrote: >Brian Starkey <brian.starkey@arm.com> writes: > >> igt_get_all_cairo_formats() allocates the format list on the heap, but >> returns it in a const pointer. Change this so that callers can free the >> array without a warning, and free the array in all callers. >> >> Signed-off-by: Brian Starkey <brian.starkey@arm.com> >> --- > >Hi, > >Assuming I'm not missing anything, I think if you free formats here, the >static variable in igt_get_all_cairo_formats() will point to garbage and >further calls to that function will return uninitialized memory. > Hi! Yes, sorry you're quite right. I'd managed to completely miss the fact the pointer is static. Please ignore this patch then, Thanks, Brian > > >-- >Gabriel Krisman Bertazi
diff --git a/lib/igt_fb.c b/lib/igt_fb.c index d2b7e9e36606..b958c970973b 100644 --- a/lib/igt_fb.c +++ b/lib/igt_fb.c @@ -1277,8 +1277,10 @@ const char *igt_format_str(uint32_t drm_format) * * This functions returns an array of all the drm fourcc codes supported by * cairo and this library. + * + * The array should be freed by the caller. */ -void igt_get_all_cairo_formats(const uint32_t **formats, int *format_count) +void igt_get_all_cairo_formats(uint32_t **formats, int *format_count) { static uint32_t *drm_formats; static int n_formats; diff --git a/lib/igt_fb.h b/lib/igt_fb.h index 4a680cefb16d..e124910367a3 100644 --- a/lib/igt_fb.h +++ b/lib/igt_fb.h @@ -154,7 +154,7 @@ int igt_cairo_printf_line(cairo_t *cr, enum igt_text_align align, uint32_t igt_bpp_depth_to_drm_format(int bpp, int depth); uint32_t igt_drm_format_to_bpp(uint32_t drm_format); const char *igt_format_str(uint32_t drm_format); -void igt_get_all_cairo_formats(const uint32_t **formats, int *format_count); +void igt_get_all_cairo_formats(uint32_t **formats, int *format_count); #endif /* __IGT_FB_H__ */ diff --git a/tests/kms_atomic.c b/tests/kms_atomic.c index 6375fede7179..9c03f6e21ebb 100644 --- a/tests/kms_atomic.c +++ b/tests/kms_atomic.c @@ -807,7 +807,7 @@ static void atomic_state_free(struct kms_atomic_state *state) static uint32_t plane_get_igt_format(struct kms_atomic_plane_state *plane) { drmModePlanePtr plane_kms; - const uint32_t *igt_formats; + uint32_t *igt_formats; uint32_t ret = 0; int num_igt_formats; int i; @@ -827,6 +827,7 @@ static uint32_t plane_get_igt_format(struct kms_atomic_plane_state *plane) } } + free(igt_formats); drmModeFreePlane(plane_kms); return ret; } diff --git a/tests/kms_render.c b/tests/kms_render.c index fd33dfb7cafe..bc2ffc750c67 100644 --- a/tests/kms_render.c +++ b/tests/kms_render.c @@ -176,7 +176,7 @@ static void test_connector(const char *test_name, struct kmstest_connector_config *cconf, enum test_flags flags) { - const uint32_t *formats; + uint32_t *formats; int format_count; int i; @@ -193,6 +193,8 @@ static void test_connector(const char *test_name, cconf, &cconf->connector->modes[0], formats[i], flags); } + + free(formats); } static int run_test(const char *test_name, enum test_flags flags)
igt_get_all_cairo_formats() allocates the format list on the heap, but returns it in a const pointer. Change this so that callers can free the array without a warning, and free the array in all callers. Signed-off-by: Brian Starkey <brian.starkey@arm.com> --- lib/igt_fb.c | 4 +++- lib/igt_fb.h | 2 +- tests/kms_atomic.c | 3 ++- tests/kms_render.c | 4 +++- 4 files changed, 9 insertions(+), 4 deletions(-)