From 5bf2271eb40bae65db24df856bac8dfe2fc91adc Mon Sep 17 00:00:00 2001 From: Yevgen Muntyan <17531749+muntyan@users.noreply.github.com> Date: Wed, 31 May 2006 22:58:15 -0500 Subject: [PATCH] Do set error in functions which take GError** argument --- moo/mooedit/mootextsearch.c | 2 +- moo/mooterm/mooterm.h | 7 +- moo/mooterm/mootermpt-cygwin.c | 80 ++++++++++-------- moo/mooterm/mootermpt-unix.c | 46 ++++++---- moo/mooutils/eggregex.c | 102 +++++++++++++---------- moo/mooutils/eggregex.h | 3 +- moo/mooutils/moocmd.c | 1 - moo/mooutils/moofileview/moofilesystem.c | 53 +++++++++--- moo/mooutils/moomarkup.c | 3 +- 9 files changed, 186 insertions(+), 111 deletions(-) diff --git a/moo/mooedit/mootextsearch.c b/moo/mooedit/mootextsearch.c index 90ab065b..4123add5 100644 --- a/moo/mooedit/mootextsearch.c +++ b/moo/mooedit/mootextsearch.c @@ -236,7 +236,7 @@ get_regex (const char *pattern, return NULL; } - egg_regex_optimize (saved_regex, error); + egg_regex_optimize (saved_regex, NULL); } return saved_regex; diff --git a/moo/mooterm/mooterm.h b/moo/mooterm/mooterm.h index 44afec05..048edd69 100644 --- a/moo/mooterm/mooterm.h +++ b/moo/mooterm/mooterm.h @@ -76,7 +76,12 @@ struct _MooTermCommand { char **argv; }; -#define MOO_TERM_ERROR (moo_term_error_quark()) +typedef enum { + MOO_TERM_ERROR_FAILED, + MOO_TERM_ERROR_INVAL +} MooTermError; + +#define MOO_TERM_ERROR (moo_term_error_quark()) GQuark moo_term_error_quark (void) G_GNUC_CONST; GType moo_term_get_type (void) G_GNUC_CONST; diff --git a/moo/mooterm/mootermpt-cygwin.c b/moo/mooterm/mootermpt-cygwin.c index ecfe17eb..478f1118 100644 --- a/moo/mooterm/mootermpt-cygwin.c +++ b/moo/mooterm/mootermpt-cygwin.c @@ -180,9 +180,8 @@ fork_command (MooTermPt *pt_gen, if (!result) return FALSE; - else - g_message ("%s: started helper pid %d", G_STRLOC, - (int) pt->pid); + + g_message ("%s: started helper pid %d", G_STRLOC, (int) pt->pid); pt->in_io = g_io_channel_win32_new_fd (pt->in); g_io_channel_set_encoding (pt->in_io, NULL, NULL); @@ -538,28 +537,28 @@ run_in_helper (const char *cmd, if(_pipe (helper_in, 512, O_NOINHERIT | O_BINARY) == -1) { - g_set_error (error, MOO_TERM_ERROR, errno, + g_set_error (error, MOO_TERM_ERROR, MOO_TERM_ERROR_FAILED, "_pipe: %s", g_strerror (errno)); goto error; } if(_pipe (helper_out, 512, O_NOINHERIT | O_BINARY) == -1) { - g_set_error (error, MOO_TERM_ERROR, errno, + g_set_error (error, MOO_TERM_ERROR, MOO_TERM_ERROR_FAILED, "_pipe: %s", g_strerror (errno)); goto error; } if (_dup2 (helper_in[READ_END], 0)) { - g_set_error (error, MOO_TERM_ERROR, errno, + g_set_error (error, MOO_TERM_ERROR, MOO_TERM_ERROR_FAILED, "_dup2: %s", g_strerror (errno)); goto error; } if (_dup2 (helper_out[WRITE_END], 1)) { - g_set_error (error, MOO_TERM_ERROR, errno, + g_set_error (error, MOO_TERM_ERROR, MOO_TERM_ERROR_FAILED, "_dup2: %s", g_strerror (errno)); goto error; } @@ -579,10 +578,10 @@ run_in_helper (const char *cmd, // NULL); #endif - /************************************************************************ + /* * We need to use CreateProcess here in order to be able to create new, * hidden console - */ + */ if (!HELPER_DIR || !HELPER_DIR[0]) { @@ -676,15 +675,13 @@ run_in_helper (const char *cmd, sinfo.wShowWindow = SW_HIDE; if (! CreateProcess (helper_binary->str, cmd_line, NULL, NULL, TRUE, - CREATE_NEW_CONSOLE | CREATE_NEW_PROCESS_GROUP, - NULL, - HELPER_DIR, - &sinfo, - &pinfo)) + CREATE_NEW_CONSOLE | CREATE_NEW_PROCESS_GROUP, + NULL, HELPER_DIR, &sinfo, &pinfo)) { - g_set_error (error, MOO_TERM_ERROR, 0, - "CreateProcess: %s", - g_win32_error_message (GetLastError ())); + char *msg = g_win32_error_message (GetLastError ()); + g_set_error (error, MOO_TERM_ERROR, MOO_TERM_ERROR_FAILED, + "CreateProcess: %s", msg); + g_free (msg); goto error; } @@ -797,10 +794,12 @@ send_intr (MooTermPt *pt) gboolean _moo_term_check_cmd (MooTermCommand *cmd, - G_GNUC_UNUSED GError **error) + GError **error) { + GString *cmd_line = NULL; + char **p; + g_return_val_if_fail (cmd != NULL, FALSE); - g_return_val_if_fail (cmd->cmd_line != NULL || cmd->argv != NULL, FALSE); if (cmd->cmd_line) { @@ -808,24 +807,31 @@ _moo_term_check_cmd (MooTermCommand *cmd, cmd->argv = NULL; return TRUE; } - else + + if (!cmd->argv) { - GString *cmd_line = NULL; - char **p; - - g_return_val_if_fail (cmd->argv[0] != NULL, FALSE); - - cmd_line = g_string_new (""); - - for (p = cmd->argv; *p != NULL; ++p) - { - if (strchr (*p, ' ')) - g_string_append_printf (cmd_line, "\"%s\" ", *p); - else - g_string_append_printf (cmd_line, "%s ", *p); - } - - cmd->cmd_line = g_string_free (cmd_line, FALSE); - return TRUE; + g_set_error (error, MOO_TERM_ERROR, MOO_TERM_ERROR_INVAL, + "NULL command line and arguments array"); + return FALSE; } + + if (!cmd->argv[0]) + { + g_set_error (error, MOO_TERM_ERROR, MOO_TERM_ERROR_INVAL, + "empty arguments array"); + return FALSE; + } + + cmd_line = g_string_new (""); + + for (p = cmd->argv; *p != NULL; ++p) + { + if (strchr (*p, ' ')) + g_string_append_printf (cmd_line, "\"%s\" ", *p); + else + g_string_append_printf (cmd_line, "%s ", *p); + } + + cmd->cmd_line = g_string_free (cmd_line, FALSE); + return TRUE; } diff --git a/moo/mooterm/mootermpt-unix.c b/moo/mooterm/mootermpt-unix.c index caa69c30..ec8b4a72 100644 --- a/moo/mooterm/mootermpt-unix.c +++ b/moo/mooterm/mootermpt-unix.c @@ -168,11 +168,12 @@ static void set_size (MooTermPt *pt, } -static gboolean fork_argv (MooTermPt *pt_gen, - char **argv, - const char *working_dir, - char **envp, - GError **error) +static gboolean +fork_argv (MooTermPt *pt_gen, + char **argv, + const char *working_dir, + char **envp, + GError **error) { MooTermPtUnix *pt; int env_len = 0; @@ -181,9 +182,16 @@ static gboolean fork_argv (MooTermPt *pt_gen, int i; GSource *src; - g_return_val_if_fail (argv != NULL && argv[0] != NULL, FALSE); + g_return_val_if_fail (argv != NULL, FALSE); g_return_val_if_fail (MOO_IS_TERM_PT_UNIX (pt_gen), FALSE); + if (!argv[0]) + { + g_set_error (error, MOO_TERM_ERROR, MOO_TERM_ERROR_INVAL, + "empty arguments array"); + return FALSE; + } + pt = MOO_TERM_PT_UNIX (pt_gen); g_return_val_if_fail (!pt_gen->priv->child_alive, FALSE); @@ -216,11 +224,13 @@ static gboolean fork_argv (MooTermPt *pt_gen, g_critical ("%s: could not fork child", G_STRLOC); if (errno) - g_set_error (error, MOO_TERM_ERROR, errno, + g_set_error (error, MOO_TERM_ERROR, + MOO_TERM_ERROR_FAILED, "could not fork command: %s", g_strerror (errno)); else - g_set_error (error, MOO_TERM_ERROR, errno, + g_set_error (error, MOO_TERM_ERROR, + MOO_TERM_ERROR_FAILED, "could not fork command"); return FALSE; @@ -701,8 +711,7 @@ static char *argv_to_cmd_line (char **argv) GString *cmd = NULL; char **p; - g_return_val_if_fail (argv != NULL, NULL); - g_return_val_if_fail (argv[0] != NULL, NULL); + g_return_val_if_fail (argv && argv[0], NULL); for (p = argv; *p != NULL; ++p) { @@ -740,14 +749,21 @@ _moo_term_check_cmd (MooTermCommand *cmd, if (cmd->argv) { + if (!cmd->argv[0]) + { + g_set_error (error, MOO_TERM_ERROR, MOO_TERM_ERROR_INVAL, + "empty arguments array"); + return FALSE; + } + g_free (cmd->cmd_line); cmd->cmd_line = argv_to_cmd_line (cmd->argv); + g_return_val_if_fail (cmd->cmd_line != NULL, FALSE); + return TRUE; } - else - { - cmd->argv = cmd_line_to_argv (cmd->cmd_line, error); - return cmd->argv != NULL; - } + + cmd->argv = cmd_line_to_argv (cmd->cmd_line, error); + return cmd->argv != NULL; } diff --git a/moo/mooutils/eggregex.c b/moo/mooutils/eggregex.c index 5d8257d1..17af6d0d 100644 --- a/moo/mooutils/eggregex.c +++ b/moo/mooutils/eggregex.c @@ -45,6 +45,8 @@ * 10/03/2005: removed #include "config.h", removed odd 'break' after 'goto' to * avoid warning * 04/25/2006: made egg_regex_new return NULL on error + * 05/31/2006: made egg_regex_optimize return boolean, cleaned up places which + * set GError * * mooutils/eggregex.c *****************************************************************************/ @@ -110,14 +112,16 @@ egg_regex_new (const gchar *pattern, EggRegexMatchFlags match_options, GError **error) { - EggRegex *regex = g_new0 (EggRegex, 1); + EggRegex *regex; const gchar *errmsg; gint erroffset; gint capture_count; + /* make pcre use glib memory functions */ pcre_malloc = (gpointer (*) (size_t)) g_malloc; pcre_free = g_free; + regex = g_new0 (EggRegex, 1); regex->ref_count = 1; /* preset the parts of gregex that need to be set, regardless of the @@ -233,26 +237,28 @@ egg_regex_clear (EggRegex *regex) * If the pattern will be used many times, then it may be worth the * effort to optimize it to improve the speed of matches. */ -void +gboolean egg_regex_optimize (EggRegex *regex, - GError **error) + GError **error) { const gchar *errmsg; - g_return_if_fail (regex != NULL && regex->regex != NULL); + g_return_val_if_fail (regex && regex->regex, FALSE); regex->extra = pcre_study (regex->regex, 0, &errmsg); if (errmsg) - { - GError *tmp_error = g_error_new (EGG_REGEX_ERROR, - EGG_REGEX_ERROR_OPTIMIZE, - _("Error while optimizing " - "regular expression %s: %s"), - regex->pattern, - errmsg); - g_propagate_error (error, tmp_error); - } + { + g_set_error (error, EGG_REGEX_ERROR, + EGG_REGEX_ERROR_OPTIMIZE, + _("Error while optimizing " + "regular expression %s: %s"), + regex->pattern, + errmsg); + return FALSE; + } + + return TRUE; } /** @@ -703,7 +709,6 @@ expand_escape (const gchar *replacement, gint x, d, h, i; const gchar *error_detail; gint base = 0; - GError *tmp_error = NULL; p++; switch (*p) @@ -918,16 +923,14 @@ expand_escape (const gchar *replacement, return p; - error: - tmp_error = g_error_new (EGG_REGEX_ERROR, - EGG_REGEX_ERROR_REPLACE, - _("Error while parsing replacement " - "text \"%s\" at char %d: %s"), - replacement, - p - replacement, - error_detail); - g_propagate_error (error, tmp_error); - +error: + g_set_error (error, EGG_REGEX_ERROR, + EGG_REGEX_ERROR_REPLACE, + _("Error while parsing replacement " + "text \"%s\" at char %d: %s"), + replacement, + p - replacement, + error_detail); return NULL; } @@ -946,11 +949,10 @@ split_replacement (const gchar *replacement, { data = g_new0 (InterpolationData, 1); start = p = expand_escape (replacement, p, data, error); - if (*error) + if (!start) { g_list_foreach (list, (GFunc)free_interpolation_data, NULL); g_list_free (list); - return NULL; } list = g_list_prepend (list, data); @@ -975,7 +977,7 @@ split_replacement (const gchar *replacement, } static gboolean -interpolate_replacement (EggRegex *regex, +interpolate_replacement (EggRegex *regex, const gchar *string, GString *result, gpointer data) @@ -1035,25 +1037,34 @@ interpolate_replacement (EggRegex *regex, * Returns: a newly allocated string containing the replacements. */ gchar * -egg_regex_replace (EggRegex *regex, - const gchar *string, - gssize string_len, - const gchar *replacement, - EggRegexMatchFlags match_options, - GError **error) +egg_regex_replace (EggRegex *regex, + const gchar *string, + gssize string_len, + const gchar *replacement, + EggRegexMatchFlags match_options, + GError **error) { gchar *result; GList *list; + GError *error_here = NULL; + + list = split_replacement (replacement, &error_here); + + if (error_here) + { + g_propagate_error (error, error_here); + return NULL; + } - list = split_replacement (replacement, error); result = egg_regex_replace_eval (regex, - string, string_len, - interpolate_replacement, - (gpointer)list, - match_options); - g_list_foreach (list, (GFunc)free_interpolation_data, NULL); - g_list_free (list); + string, + string_len, + interpolate_replacement, + list, + match_options); + g_list_foreach (list, (GFunc) free_interpolation_data, NULL); + g_list_free (list); return result; } @@ -1139,7 +1150,7 @@ egg_regex_eval_replacement (EggRegex *regex, result = g_string_new (NULL); interpolate_replacement (regex, string, result, list); - g_list_foreach (list, (GFunc)free_interpolation_data, NULL); + g_list_foreach (list, (GFunc) free_interpolation_data, NULL); g_list_free (list); return g_string_free (result, FALSE); @@ -1185,13 +1196,13 @@ egg_regex_check_replacement (const gchar *replacement, } } - g_list_foreach (list, (GFunc)free_interpolation_data, NULL); + g_list_foreach (list, (GFunc) free_interpolation_data, NULL); g_list_free (list); return TRUE; } -gchar* +gchar * egg_regex_try_eval_replacement (EggRegex *regex, const gchar *replacement, GError **error) @@ -1235,6 +1246,11 @@ egg_regex_try_eval_replacement (EggRegex *regex, break; case REPL_TYPE_NUMERIC_REFERENCE: case REPL_TYPE_SYMBOLIC_REFERENCE: + g_set_error (error, EGG_REGEX_ERROR, + EGG_REGEX_ERROR_REPLACE, + "Pattern '%s' contains internal references, can't " + "evaluate replacement without matching", + egg_regex_get_pattern (regex)); result = FALSE; break; } diff --git a/moo/mooutils/eggregex.h b/moo/mooutils/eggregex.h index c6c35146..307a8fa8 100644 --- a/moo/mooutils/eggregex.h +++ b/moo/mooutils/eggregex.h @@ -40,6 +40,7 @@ * * 04/24/2005: added refcounting * 04/30/2005: added egg_regex_eval_replacement and egg_regex_check_replacement + * 05/31/2006: made egg_regex_optimize return boolean * * mooutils/eggregex.h *****************************************************************************/ @@ -107,7 +108,7 @@ EggRegex *egg_regex_new (const gchar *pattern, EggRegexCompileFlags compile_options, EggRegexMatchFlags match_options, GError **error); -void egg_regex_optimize (EggRegex *regex, +gboolean egg_regex_optimize (EggRegex *regex, GError **error); /* ref() and unref() accept NULL */ EggRegex *egg_regex_ref (EggRegex *regex); diff --git a/moo/mooutils/moocmd.c b/moo/mooutils/moocmd.c index 37bfefc8..f386d8eb 100644 --- a/moo/mooutils/moocmd.c +++ b/moo/mooutils/moocmd.c @@ -490,7 +490,6 @@ moo_cmd_run_command (MooCmd *cmd, g_return_val_if_fail (MOO_IS_CMD (cmd), FALSE); g_return_val_if_fail (argv && argv[0], FALSE); - g_return_val_if_fail (!cmd->priv->running, FALSE); #ifdef __WIN32__ diff --git a/moo/mooutils/moofileview/moofilesystem.c b/moo/mooutils/moofileview/moofilesystem.c index af7c45d1..a5d2aa57 100644 --- a/moo/mooutils/moofileview/moofilesystem.c +++ b/moo/mooutils/moofileview/moofilesystem.c @@ -352,7 +352,6 @@ folder_deleted (MooFolder *folder, } -/* XXX check setting error */ MooFolder * get_folder (MooFileSystem *fs, const char *path, @@ -365,14 +364,19 @@ get_folder (MooFileSystem *fs, g_return_val_if_fail (path != NULL, NULL); #ifdef __WIN32__ - if (!strcmp (path, "")) - { - g_clear_error (error); + if (!*path) return get_root_folder_win32 (fs, wanted); - } #endif /* __WIN32__ */ - g_return_val_if_fail (g_path_is_absolute (path), NULL); + /* XXX check the caller */ + if (!g_path_is_absolute (path)) + { + g_set_error (error, MOO_FILE_ERROR, + MOO_FILE_ERROR_BAD_FILENAME, + "folder path '%s' is not absolute", + path); + return NULL; + } norm_path = _moo_file_system_normalize_path (fs, path, TRUE, error); @@ -431,7 +435,16 @@ create_folder (G_GNUC_UNUSED MooFileSystem *fs, GError **error) { g_return_val_if_fail (path != NULL, FALSE); - g_return_val_if_fail (g_path_is_absolute (path), FALSE); + + /* XXX check the caller */ + if (!g_path_is_absolute (path)) + { + g_set_error (error, MOO_FILE_ERROR, + MOO_FILE_ERROR_BAD_FILENAME, + "folder path '%s' is not absolute", + path); + return FALSE; + } /* TODO mkdir must (?) adjust permissions according to umask */ #ifndef __WIN32__ @@ -442,8 +455,8 @@ create_folder (G_GNUC_UNUSED MooFileSystem *fs, { int saved_errno = errno; g_set_error (error, MOO_FILE_ERROR, - moo_file_error_from_errno (saved_errno), - "%s", g_strerror (saved_errno)); + moo_file_error_from_errno (saved_errno), + "%s", g_strerror (saved_errno)); return FALSE; } @@ -647,9 +660,18 @@ make_path_unix (G_GNUC_UNUSED MooFileSystem *fs, char *path, *name; g_return_val_if_fail (base_path != NULL, NULL); - g_return_val_if_fail (g_path_is_absolute (base_path), NULL); g_return_val_if_fail (display_name != NULL, NULL); + /* XXX check the caller */ + if (!g_path_is_absolute (base_path)) + { + g_set_error (error, MOO_FILE_ERROR, + MOO_FILE_ERROR_BAD_FILENAME, + "path '%s' is not absolute", + base_path); + return NULL; + } + name = g_filename_from_utf8 (display_name, -1, NULL, NULL, &error_here); if (error_here) @@ -726,7 +748,16 @@ parse_path_unix (MooFileSystem *fs, char *display_dirname = NULL, *display_basename = NULL; g_return_val_if_fail (path_utf8 && path_utf8[0], FALSE); - g_return_val_if_fail (g_path_is_absolute (path_utf8), FALSE); + + /* XXX check the caller */ + if (!g_path_is_absolute (path_utf8)) + { + g_set_error (error, MOO_FILE_ERROR, + MOO_FILE_ERROR_BAD_FILENAME, + "path '%s' is not absolute", + path_utf8); + return FALSE; + } if (!strcmp (path_utf8, "/")) { diff --git a/moo/mooutils/moomarkup.c b/moo/mooutils/moomarkup.c index 93f8f463..b7e10c64 100644 --- a/moo/mooutils/moomarkup.c +++ b/moo/mooutils/moomarkup.c @@ -131,7 +131,8 @@ moo_markup_parse_memory (const char *buffer, ParserState state; GMarkupParseContext *context; - if (size < 0) size = strlen (buffer); + if (size < 0) + size = strlen (buffer); doc = moo_markup_doc_new_priv (NULL); state.doc = doc;