From 47cdb532f191a0788804db4806b90abe7ec28a1f Mon Sep 17 00:00:00 2001 From: Frank Praznik Date: Thu, 2 Feb 2023 13:43:43 -0500 Subject: [PATCH] video: Don't rely on memcpy undefined behavior The C specification states that passing a size of 0 to functions like memcpy is valid, but even if the size is 0 and the function is essentially a no-op, the result when passing any invalid pointers is considered undefined behavior. Don't rely on undefined behavior when copying the display or mode lists. --- src/video/SDL_video.c | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/src/video/SDL_video.c b/src/video/SDL_video.c index e5b479519..45878148a 100644 --- a/src/video/SDL_video.c +++ b/src/video/SDL_video.c @@ -634,17 +634,19 @@ SDL_DisplayID SDL_AddVideoDisplay(const SDL_VideoDisplay *display, SDL_bool send if (displays) { int i; - /* The display list may contain self-referential pointers to the desktop mode. */ - SDL_memcpy(displays, _this->displays, _this->num_displays * sizeof(*displays)); - for (i = 0; i < _this->num_displays; ++i) { - if (displays[i].current_mode == &_this->displays[i].desktop_mode) { - displays[i].current_mode = &displays[i].desktop_mode; + if (_this->displays) { + /* The display list may contain self-referential pointers to the desktop mode. */ + SDL_memcpy(displays, _this->displays, _this->num_displays * sizeof(*displays)); + for (i = 0; i < _this->num_displays; ++i) { + if (displays[i].current_mode == &_this->displays[i].desktop_mode) { + displays[i].current_mode = &displays[i].desktop_mode; + } } + + SDL_free(_this->displays); } - SDL_free(_this->displays); _this->displays = displays; - id = _this->next_object_id++; new_display = &displays[_this->num_displays++]; @@ -954,15 +956,18 @@ SDL_bool SDL_AddFullscreenDisplayMode(SDL_VideoDisplay *display, const SDL_Displ return SDL_FALSE; } - /* Copy the list and update the current mode pointer, if necessary. */ - SDL_memcpy(modes, display->fullscreen_modes, nmodes * sizeof(*modes)); - for (i = 0; i < nmodes; ++i) { - if (display->current_mode == &display->fullscreen_modes[i]) { - display->current_mode = &modes[i]; + if (display->fullscreen_modes) { + /* Copy the list and update the current mode pointer, if necessary. */ + SDL_memcpy(modes, display->fullscreen_modes, nmodes * sizeof(*modes)); + for (i = 0; i < nmodes; ++i) { + if (display->current_mode == &display->fullscreen_modes[i]) { + display->current_mode = &modes[i]; + } } + + SDL_free(display->fullscreen_modes); } - SDL_free(display->fullscreen_modes); display->fullscreen_modes = modes; display->max_fullscreen_modes += 32; }