Skip to content

Conversation

icculus
Copy link
Collaborator

@icculus icculus commented Jul 14, 2025

Fixes #13322.

@icculus icculus added this to the 3.4.0 milestone Jul 14, 2025
@icculus icculus self-assigned this Jul 14, 2025
@@ -107,12 +99,7 @@
result = (char *)SDL_malloc(len);
if (result != NULL) {
char *ptr;

Choose a reason for hiding this comment

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

ptr outside of the loop becomes unused.

@@ -42,27 +42,13 @@ char *SDL_SYS_GetPrefPath(const char *org, const char *app)
const char *append = "/libsdl/";
char *result;
char *ptr = NULL;

Choose a reason for hiding this comment

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

Like the above change, ptr is only used in the for loop.

result can be defined below as char* result = (char *)SDL_malloc(len);; same for result in many other implementations. (Or like other implementations, may be defined as char *result = NULL;).

SDL_InvalidParamError("app");
return NULL;
}

pref_path = MakePrefPath(app);

Choose a reason for hiding this comment

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

Can be char *pref_path = MakePrefPath(app);

@@ -80,15 +80,6 @@ char *SDL_SYS_GetPrefPath(const char *org, const char *app)
char *result = NULL;
size_t len;

Choose a reason for hiding this comment

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

Can be defined below as const size_t len = SDL_strlen(base) + SDL_strlen(org) + SDL_strlen(app) + 4;

@@ -269,15 +269,6 @@ char *SDL_SYS_GetPrefPath(const char *org, const char *app)
const char *append;
char *result = NULL;
char *ptr = NULL;

Choose a reason for hiding this comment

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

ptr is used only in the for loop.

SDL_snprintf(result, len, "%s%s/", base, app);
}

SDL_snprintf(result, len, "%s%s/%s/", base, org, app);
mkdir(result, 0755);

Choose a reason for hiding this comment

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

Whether to check for successfulness for mkdir-like functions? I notice some implementations check while some not...

@@ -33,7 +33,7 @@ char *SDL_SYS_GetBasePath(void)
char *SDL_SYS_GetPrefPath(const char *org, const char *app)
{
char *pref_path = NULL;
if (SDL_asprintf(&pref_path, "C:/System/Apps/%s/%s/", org ? org : "SDL_App", app) < 0) {
if (SDL_asprintf(&pref_path, "C:/System/Apps/%s/%s/", org, app) < 0) {
return NULL;
}
return pref_path;

Choose a reason for hiding this comment

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

There is no mkdir-like call for this implementation, is it correct? (I'm not familiar with the background.)

@@ -269,15 +269,6 @@ char *SDL_SYS_GetPrefPath(const char *org, const char *app)
const char *append;

Choose a reason for hiding this comment

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

May init to NULL.

@@ -46,29 +46,13 @@ char *SDL_SYS_GetPrefPath(const char *org, const char *app)
const char *envr = "ux0:/data/";
char *result = NULL;
char *ptr = NULL;

Choose a reason for hiding this comment

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

ptr is used only in the for loop.

@po-87
Copy link

po-87 commented Jul 16, 2025

Doesn't this break existing apps that relied on the old behaviour?

@python-b5
Copy link

I intentionally set org to NULL, as I dislike having another nested folder with only a single game in it anyway. This change would cause my app to start using another, incorrect folder, from what I can tell, so it would no longer find files that were already in the old folder. While it is perhaps more "correct" to specify an organization, I don't think SDL should enforce that opinion on end users, especially as a breaking change (and, nobody would actually want their app's data in a generic SDL_App folder, right...?). I would probably have to stop using this function if this were merged, and either use another library or implement something similar myself.

@ScolderCreations
Copy link
Contributor

This isn't really a good idea. Plenty of Windows applications use just a single folder in AppData rather than nesting, so this would prevent anyone from making an application like that.

@icculus icculus closed this Jul 16, 2025
@icculus icculus deleted the sdl3-getprefpath-defaults branch July 16, 2025 21:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Question: is it allowed to omit org name for SDL_GetPrefPath?
5 participants