Re: [hatari-devel] SDL GUI file and directory selection

[Andreas Grabher <andreas_g86@xxxxxxxxxx>]

Hello Eero,

thank you very much for the changes. Using Str_Copy() makes sense. May I suggest to change SdlGui_DirConfSelect() to SDLGui_DirConfSelect() for naming consistency?

I got one last question:
I noticed that File_MakeValidPathName() outputs just '\0‘ if the input is "/somefile" (some file selected in root directory). I think it should return "/" instead. It seems that the if-statement "if (pPathName[0])“ is causing that issue. I tried removing it and didn’t see any regression yet. Does anyone have an idea why this if-statement exists? The explanation of the function tells that empty strings should be left as-is. What does this mean? At least it does not seem to work properly.

Best wishes,
Andreas



> Am 18.10.2022 um 01:36 schrieb Eero Tamminen <oak@xxxxxxxxxxxxxx>:
> 
> Hi Andreas,
> 
> I added the changes you suggested.
> 
> On 15.10.2022 16.21, Andreas Grabher wrote:
>> Path is always valid (or '\0' in case of /somefile) after File_MakeValidPathName(). So SDLGui_DirSelect() can be further simplified like this:
>> bool SDLGui_DirSelect(char *dlgname, char *confname, int maxlen)
>> {
>> 	char *selname;
>> 	selname = SDLGui_FileSelect("Choose a folder", confname, NULL, NULL, false);
>> 	if (selname)
>> 	{
>> 		File_MakeValidPathName(selname);
>> 		strncpy(confname, selname, FILENAME_MAX);
> 
> Either you:
> 1. trust selname to fit into confname, and use use strcpy(), but on all OSes FILENAME_MAX might not be enough for that
> * use strncpy() *and* zero terminate the string, or
> * use Str_Copy()
> 
> 
> => I opted for last, for consistency with the other fsel function.
> 
> 
>> 		File_ShrinkName(dlgname, selname, maxlen);
>> 		free(selname);
>> 		return true;
>> 	}
>> 	return false;
>> }
> ...
>>> I also suggest to rename SdlGui_DirSelect() to SDLGui_DirSelect() for matching the naming scheme of the other functions.
> 
> Added "Conf" to function name for consistency.
> 
> 
> 	- Eero
> 
> 
>