Skip to content

Commit

Permalink
Added SDL_revcpy, which isn't _actually_ in SDL2.
Browse files Browse the repository at this point in the history
  • Loading branch information
icculus committed Feb 21, 2019
1 parent bb8b62e commit 4c5e3b2
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 1 deletion.
11 changes: 11 additions & 0 deletions src/SDL12_compat.c
Expand Up @@ -834,6 +834,17 @@ SDL_snprintf(char *text, size_t maxlen, const char *fmt, ...)
return retval;
}

DECLSPEC void * SDLCALL
SDL_revcpy(void *dst, const void *src, size_t len)
{
/* this doesn't reverse the data...I think this was just a memcpy that
was meant to be CPU-cache friendly if you knew you were working with
data going backwards in memory, instead of jumping over pages to copy
from the start...? Whatever, just do a memcpy here. */
return SDL_memcpy(dst, src, len);
}


DECLSPEC SDL_bool SDLCALL
SDL_HasMMXExt(void)
{
Expand Down
1 change: 0 additions & 1 deletion src/SDL20_syms.h
Expand Up @@ -220,7 +220,6 @@ SDL20_SYM_PASSTHROUGH(char *,getenv,(const char *a),(a),return)
SDL20_SYM_PASSTHROUGH(void,qsort,(void *a, size_t b, size_t c, int (*d)(const void *, const void *)),(a,b,c,d),)
SDL20_SYM_PASSTHROUGH(void *,memset,(void *a, int b, size_t c),(a,b,c),return)
SDL20_SYM_PASSTHROUGH(void *,memcpy,(void *a, const void *b, size_t c),(a,b,c),return)
SDL20_SYM_PASSTHROUGH(void *,revcpy,(void *a, const void *b, size_t c),(a,b,c),return)
SDL20_SYM_PASSTHROUGH(int,memcmp,(const void *a, const void *b, size_t c),(a,b,c),return)
SDL20_SYM_PASSTHROUGH(size_t,strlen,(const char *a),(a),return)
SDL20_SYM_PASSTHROUGH(size_t,strlcpy,(char *a, const char *b, size_t c),(a,b,c),return)
Expand Down

4 comments on commit 4c5e3b2

@kkofler
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This implementation is incorrect. The point of SDL_revcpy (reverse copy) is to copy from the end to the start rather than from the start to the end as SDL_memcpy does, which matters in case the buffers are overlapping. Compare:
https://github.com/libsdl-org/SDL-1.2/blob/b8aa08c87e3f6e87bf918e1f2e26ebabe5f07122/src/stdlib/SDL_string.c#L320
vs.
https://github.com/libsdl-org/SDL-1.2/blob/b8aa08c87e3f6e87bf918e1f2e26ebabe5f07122/src/stdlib/SDL_string.c#L276

See also the difference between memcpy and memmove in the standard C library (except that memmove automatically detects the copy direction to use whereas SDL_revcpy requires you to manually choose between SDL_memcpy and SDL_revcpy). So you either have to copy the implementation of SDL_revcpy here or call SDL_memmove, not SDL_memcpy.

@kkofler
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this bug will lead to corruption of the memory area being moved, so it needs urgent fixing.

@kkofler
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Note that SDL_memmove is also subtly different in that it does not always reverse-copy, but auto-detects the safe direction to use. I am not sure whether that can be an issue in practice. But calling SDL_memcpy always does the wrong thing if SDL_revcpy is expected and will definitely break when used instead of SDL_revcpy on overlapping buffers.)

@icculus
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be resolved in dacb4cf , thanks!

Please sign in to comment.