IntenseFinance/gnucash
3
0
Fork 0
mirror of https://github.com/Gnucash/gnucash.git synced 2025-02-25 18:55:30 -06:00
Code Issues Packages Projects Releases Wiki Activity

Bug 798458 - Build failure with gcc 12

Browse Source 
Refactor try_gz_open to return a std::pair<FILE*, thread>. That removes
the need for the threads hash-table and wait_for_gzip().

The cause of the gcc12 error was that we were using the thread's pipe's
FILE* as the key to the hash-table and had to close it before calling
wait_for_gzip(file) to remove the thread from the hash-table. gcc12
considers that a use-after-free. It happens to be wrong, but removing the
need for it results in a cleaner implementation as well as silencing the
warning.
This commit is contained in:
John Ralls 2022-03-01 12:19:44 -08:00
parent 67e8c317da
commit 81b9a02235
1 changed files with 51 additions and 87 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

124
libgnucash/backend/xml/io-gncxml-v2.cpp
View File

@ -124,11 +124,11 @@ extern const gchar*
gnc_v2_book_version_string; /* see gnc-book-xml-v2 */ gnc_v2_book_version_string; /* see gnc-book-xml-v2 */
/* Forward declarations */ /* Forward declarations */
static FILE* try_gz_open (const char* filename, const char* perms, static std::pair<FILE*, GThread*> try_gz_open (const char* filename,
const char* perms,
gboolean compress, gboolean compress,
gboolean write); gboolean write);
static gboolean is_gzipped_file (const gchar* name); static bool is_gzipped_file (const gchar* name);
static gboolean wait_for_gzip (FILE* file);
static void static void
clear_up_account_commodity ( clear_up_account_commodity (
@ -796,22 +796,21 @@ qof_session_load_from_xml_file_v2_full (
* https://bugs.gnucash.org/show_bug.cgi?id=712528 for more * https://bugs.gnucash.org/show_bug.cgi?id=712528 for more
* info. * info.
*/ */
const char* filename = xml_be->get_filename(); auto filename = xml_be->get_filename();
FILE* file; auto [file, thread] = try_gz_open (filename, "r",
gboolean is_compressed = is_gzipped_file (filename); is_gzipped_file (filename), FALSE);
file = try_gz_open (filename, "r", is_compressed, FALSE); if (!file)
if (file == NULL)
{ {
PWARN ("Unable to open file %s", filename); PWARN ("Unable to open file %s", filename);
retval = FALSE; retval = false;
} }
else else
{ {
retval = gnc_xml_parse_fd (top_parser, file, retval = gnc_xml_parse_fd (top_parser, file,
generic_callback, gd, book); generic_callback, gd, book);
fclose (file); fclose (file);
if (is_compressed) if (thread)
wait_for_gzip (file); g_thread_join (thread) != nullptr;
} }
} }
@ -1538,7 +1537,7 @@ cleanup_gz_thread_func:
return GINT_TO_POINTER (success); return GINT_TO_POINTER (success);
} }
static FILE* static std::pair<FILE*, GThread*>
try_gz_open (const char* filename, const char* perms, gboolean compress, try_gz_open (const char* filename, const char* perms, gboolean compress,
gboolean write) gboolean write)
{ {
@ -1546,11 +1545,11 @@ try_gz_open (const char* filename, const char* perms, gboolean compress,
compress = TRUE; compress = TRUE;
if (!compress) if (!compress)
return g_fopen (filename, perms); return std::pair<FILE*, GThread*>(g_fopen (filename, perms),
nullptr);
{ {
int filedes[2]{}; int filedes[2]{};
GThread* thread;
gz_thread_params_t* params; gz_thread_params_t* params;
FILE* file; FILE* file;
@ -1575,7 +1574,8 @@ try_gz_open (const char* filename, const char* perms, gboolean compress,
close(filedes[0]); close(filedes[0]);
close(filedes[1]); close(filedes[1]);
} }
return g_fopen (filename, perms); return std::pair<FILE*, GThread*>(g_fopen (filename, perms),
nullptr);
} }
params = g_new (gz_thread_params_t, 1); params = g_new (gz_thread_params_t, 1);
@ -1584,7 +1584,7 @@ try_gz_open (const char* filename, const char* perms, gboolean compress,
params->perms = g_strdup (perms); params->perms = g_strdup (perms);
params->write = write; params->write = write;
thread = g_thread_new ("xml_thread", (GThreadFunc) gz_thread_func, auto thread = g_thread_new ("xml_thread", (GThreadFunc) gz_thread_func,
params); params);
if (!thread) if (!thread)
{ {
@ -1595,70 +1595,38 @@ try_gz_open (const char* filename, const char* perms, gboolean compress,
close (filedes[0]); close (filedes[0]);
close (filedes[1]); close (filedes[1]);
return g_fopen (filename, perms);
} }
else
{
if (write) if (write)
file = fdopen (filedes[1], "w"); file = fdopen (filedes[1], "w");
else else
file = fdopen (filedes[0], "r"); file = fdopen (filedes[0], "r");
G_LOCK (threads);
if (!threads)
threads = g_hash_table_new (g_direct_hash, g_direct_equal);
g_hash_table_insert (threads, file, thread);
G_UNLOCK (threads);
return file;
} }
}
static gboolean return std::pair<FILE*, GThread*>(file, thread);
wait_for_gzip (FILE* file)
{
gboolean retval = TRUE;
G_LOCK (threads);
if (threads)
{
GThread* thread = static_cast<decltype (thread)> (g_hash_table_lookup (threads,
file));
if (thread)
{
g_hash_table_remove (threads, file);
retval = GPOINTER_TO_INT (g_thread_join (thread));
} }
}
G_UNLOCK (threads);
return retval;
} }
gboolean gboolean
gnc_book_write_to_xml_file_v2 ( gnc_book_write_to_xml_file_v2 (QofBook* book, const char* filename,
QofBook* book,
const char* filename,
gboolean compress) gboolean compress)
{ {
FILE* out; bool success = true;
gboolean success = TRUE;
out = try_gz_open (filename, "w", compress, TRUE); auto [file, thread] = try_gz_open (filename, "w", compress, TRUE);
if (!file)
return false;
/* Try to write as much as possible */ /* Try to write as much as possible */
if (!out success = (gnc_book_write_to_xml_filehandle_v2 (book, file));
|| !gnc_book_write_to_xml_filehandle_v2 (book, out))
success = FALSE;
/* Close the output stream */ /* Close the output stream */
if (out && fclose (out)) success = ! (fclose (file));
success = FALSE;
/* Optionally wait for parallel compression threads */ /* Optionally wait for parallel compression threads */
if (out && compress) if (thread)
if (!wait_for_gzip (out)) success = g_thread_join (thread) != nullptr;
success = FALSE;
return success; return success;
} }
@ -1697,7 +1665,7 @@ gnc_book_write_accounts_to_xml_file_v2 (QofBackend* qof_be, QofBook* book,
} }
/***********************************************************************/ /***********************************************************************/
static gboolean static bool
is_gzipped_file (const gchar* name) is_gzipped_file (const gchar* name)
{ {
unsigned char buf[2]; unsigned char buf[2];
@ -1705,22 +1673,23 @@ is_gzipped_file (const gchar* name)
if (fd == -1) if (fd == -1)
{ {
return FALSE; return false;
} }
if (read (fd, buf, 2) != 2) if (read (fd, buf, 2) != 2)
{ {
close (fd); close (fd);
return FALSE; return false;
} }
close (fd); close (fd);
/* 037 0213 are the header id bytes for a gzipped file. */
if (buf[0] == 037 && buf[1] == 0213) if (buf[0] == 037 && buf[1] == 0213)
{ {
return TRUE; return true;
} }
return FALSE; return false;
} }
QofBookFileType QofBookFileType
@ -1845,18 +1814,16 @@ gnc_xml2_find_ambiguous (const gchar* filename, GList* encodings,
GHashTable** unique, GHashTable** ambiguous, GHashTable** unique, GHashTable** ambiguous,
GList** impossible) GList** impossible)
{ {
FILE* file = NULL;
GList* iconv_list = NULL, *conv_list = NULL, *iter; GList* iconv_list = NULL, *conv_list = NULL, *iter;
iconv_item_type* iconv_item = NULL, *ascii = NULL; iconv_item_type* iconv_item = NULL, *ascii = NULL;
const gchar* enc; const gchar* enc;
GHashTable* processed = NULL; GHashTable* processed = NULL;
gint n_impossible = 0; gint n_impossible = 0;
GError* error = NULL; GError* error = NULL;
gboolean is_compressed;
gboolean clean_return = FALSE; gboolean clean_return = FALSE;
is_compressed = is_gzipped_file (filename); auto [file, thread] = try_gz_open (filename, "r",
file = try_gz_open (filename, "r", is_compressed, FALSE); is_gzipped_file (filename), FALSE);
if (file == NULL) if (file == NULL)
{ {
PWARN ("Unable to open file %s", filename); PWARN ("Unable to open file %s", filename);
@ -2039,8 +2006,8 @@ cleanup_find_ambs:
if (file) if (file)
{ {
fclose (file); fclose (file);
if (is_compressed) if (thread)
wait_for_gzip (file); g_thread_join (thread);
} }
return (clean_return) ? n_impossible : -1; return (clean_return) ? n_impossible : -1;
@ -2056,17 +2023,14 @@ static void
parse_with_subst_push_handler (xmlParserCtxtPtr xml_context, parse_with_subst_push_handler (xmlParserCtxtPtr xml_context,
push_data_type* push_data) push_data_type* push_data)
{ {
const gchar* filename;
FILE* file = NULL;
GIConv ascii = (GIConv) - 1; GIConv ascii = (GIConv) - 1;
GString* output = NULL; GString* output = NULL;
GError* error = NULL; GError* error = NULL;
gboolean is_compressed;
filename = push_data->filename; auto filename = push_data->filename;
is_compressed = is_gzipped_file (filename); auto [file, thread] = try_gz_open (filename, "r",
file = try_gz_open (filename, "r", is_compressed, FALSE); is_gzipped_file (filename), FALSE);
if (file == NULL) if (!file)
{ {
PWARN ("Unable to open file %s", filename); PWARN ("Unable to open file %s", filename);
goto cleanup_push_handler; goto cleanup_push_handler;
@ -2179,8 +2143,8 @@ cleanup_push_handler:
if (file) if (file)
{ {
fclose (file); fclose (file);
if (is_compressed) if (thread)
wait_for_gzip (file); g_thread_join (thread);
} }
} }