[Github-comments] [geany/geany] Create project's filename path (#2586)

[Matthew Brush]

@codebrainz requested changes on this pull request.



> +	{
+		gboolean create_dir = dialogs_show_question_full(NULL, GTK_STOCK_OK, GTK_STOCK_CANCEL,
+			_("Create the project's Filename directory?"),
+			_("The path \"%s\" does not exist."),
+			project_file_dirname);
+
+		/* if the project file's directory doesn't exist and the user wants to try and create it */
+		if (create_dir)
+		{
+			/* try and create the project file's directory recursively */
+			err_code = utils_mkdir(project_file_dirname, TRUE);
+			if (err_code != 0)
+			{
+				SHOW_ERR1(_("Filename path could not be created (%s)."), g_strerror(err_code));
+				gtk_widget_grab_focus(e->file_name);
+				utils_free_pointers(2, project_file_dirname, locale_filename, NULL);

It wasn't leaking `locale_filename`, it's used in the existing code further down, so doing this will cause it to access a dangling pointer and crash at best.

> +		if (create_dir)
+		{
+			/* try and create the project file's directory recursively */
+			err_code = utils_mkdir(project_file_dirname, TRUE);
+			if (err_code != 0)
+			{
+				SHOW_ERR1(_("Filename path could not be created (%s)."), g_strerror(err_code));
+				gtk_widget_grab_focus(e->file_name);
+				utils_free_pointers(2, project_file_dirname, locale_filename, NULL);
+				return FALSE;
+			}
+		}
+		else
+		{
+			gtk_widget_grab_focus(e->file_name);
+			utils_free_pointers(2, project_file_dirname, locale_filename, NULL);

Same here for freeing `locale_filename`.

> +		else
+		{
+			gtk_widget_grab_focus(e->file_name);
+			utils_free_pointers(2, project_file_dirname, locale_filename, NULL);
+			return FALSE;
+		}

I haven't thought about this much (hence the TODO in my example code), are you sure this is the best thing to do?

I get that the path is tested above, but what if the directory is created in the meantime (ex. the user leaves the question dialog box up and create the directory using their file manager or the terminal)?

I could be wrong, but it seems like falling back to the old behaviour here would be ok, no?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/2586#pullrequestreview-489178213
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.geany.org/pipermail/github-comments/attachments/20200915/8f182707/attachment.htm>