Re: code style in the vfs

[Pavel Tsekov <ptsekov gmx net>]

Hello,

On Fri, 24 Sep 2004, Roland Illig wrote:

> I have rewritten a small part of the vfs. I noticed that the function
> _vfs_get_class -- although declared to take a "const char *" cannot be
> fed with a string literal, because it modifies the path. This has led to
> two changes:

Then this function should be declared to take `char *' as an argument.

> 1. I extended my private src/global.h (which I will not commit unless
> asked to do it):
>
> #ifdef strchr
> #undef strchr
> #endif
> #define strchr(m_str, m_chr) (const char *) (strchr) (m_str, m_chr)
> #define mod_strchr(m_str, m_chr) (strchr) (m_str, m_chr)
>
> The point is that strchr takes a "const char *", but it returns a "char
> *", which can point to the "const char *", effectively discarding the
> "const". Using these definitions I get some more warnings which I will
> fix in the next time. There are many more functions which are declared
> inappropriately, like strchr, and I will try to find them all.

I do not agree about strchr () being declared inappropriately. strchr ()
does not modify the buffer so it is ok to take `const char *'. It returns
a `char *' which is perfectly ok since it is up to the caller to decide
what to do with the return value. What I try to point out is that the
fact that a function takes const argument does not necessarily mean that
the argument it is fed should be considered non modifiable throughout the
whole program.

> 2. Many vfs functions make a local copy of a string, change only one
> character and then pass that string to other functions. For example:
>
>      slash = strchr (semi, PATH_SEP);
>      if (slash)
>          *slash = 0;
>      ret = vfs_prefix_to_class (semi + 1);
>
> This pattern requires the massive use of g_strdup, which can be
> annoying, because the string duplicates must be freed later.

I don't see anything like that in the code snippet above.

> My approach has been to change string parameters into two parameters,
> the string and the string_len. This eliminates the need for most of the
> cases like the example above.

How ? Please, show an example of what it was and what is on your box.