[PATCH] concat_dir_and_file() needs fixes
Jindrich Novy
jnovy at redhat.com
Fri Dec 2 18:11:20 UTC 2005
Hello Roland,
On Fri, 2005-12-02 at 15:58 +0100, Roland Illig wrote:
> Jindrich Novy wrote:
> > --- mc-4.6.1a/src/util.c.jn 2005-12-02 11:08:26.000000000 +0100
> > +++ mc-4.6.1a/src/util.c 2005-12-02 13:11:19.000000000 +0100
> > @@ -1515,9 +1515,16 @@
> >
> > /* If filename is NULL, then we just append PATH_SEP to the dir */
> > char *
> > -concat_dir_and_file (const char *dir, const char *file)
> > +concat_dir_and_file (const char *dir, const char *filename)
> > {
> > int i = strlen (dir);
> > + const char *file = filename;
> > +
>
> It's obvious that you used mcedit here. :) (Hint: trailing white-space.)
Yes ;)
> > + /* Return filename when dir is empty */
> > + if (!i) return g_strdup (filename);
>
> This looks almost good, except that "i" is not a boolean variable. You
> should use "if (i == 0)" instead.
It's a question of style, !i and i==0 are equivalent, but feel free to
change it.
> > +
> > + if (file != NULL && *file == PATH_SEP)
> > + file++;
>
> Maybe we should rather make sure that this function is never called with
> non-empty "dir" and "file" starting with a slash. Otherwise we might
> hide bugs. How often do you want to concatenate two absolute paths?
The problem isn't to concatenate two absolute paths actually. The patch
enhances the functionality of concat_dir_and_file() in the way that it
will correctly add say dir="/home/jnovy" and file="/.mc/bindings" to
"/home/jnovy/.mc/bindings" as the unfixed concat_dir_and_file checks
only whether the dir ends with PATH_SEP and adds it even if file begins
with PATH_SEP already -> the result is "/home/jnovy//.mc/bindings"
what's not too good. Fortunately the path works even if it's so ugly.
I thought that it would be less PITA to make concat_dir_and_file less
stupid than fixing the strings in edit.h:
/* File names */
#define EDIT_DIR PATH_SEP_STR ".mc" PATH_SEP_STR "cedit"
#define SYNTAX_FILE EDIT_DIR PATH_SEP_STR "Syntax"
#define CLIP_FILE EDIT_DIR PATH_SEP_STR "cooledit.clip"
#define MACRO_FILE EDIT_DIR PATH_SEP_STR "cooledit.macros"
#define BLOCK_FILE EDIT_DIR PATH_SEP_STR "cooledit.block"
#define TEMP_FILE EDIT_DIR PATH_SEP_STR "cooledit.temp"
used for concatenation, all of them beginning with PATH_SEP. Please note
that if you remove the leading PATH_SEP from these strings, you need to
rewrite almost all calls to catstrs(), edit_save_block() and friends by
at least adding the PATH_SEP_STR there.
Regards,
Jindrich
--
Jindrich Novy <jnovy at redhat.com>, http://people.redhat.com/jnovy/
(o_ _o)
//\ The worst evil in the world is refusal to think. //\
V_/_ _\_V
More information about the mc-devel
mailing list