[Devel] General handling of special cases if perferred
Kir Kolyshkin
kir at openvz.org
Sun Apr 28 21:43:07 PDT 2013
On 04/28/2013 08:31 PM, Igor Podlesny wrote:
> ---
> src/lib/util.c | 19 +++++++++----------
> 1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/src/lib/util.c b/src/lib/util.c
> index 8cc066b..420a574 100644
> --- a/src/lib/util.c
> +++ b/src/lib/util.c
> @@ -46,18 +46,17 @@ static const char *unescapestr(char *const src)
> char *p1, *p2;
> int fl = 0;
>
> - if (src == NULL)
> - return NULL;
I don't like this approach.
In current version, a reviewer don't have to read the whole function
for the case when src is NULL, and it's clear we don't do anything in
that case.
In your version, he have to read it in it's entirety, and it's a bit
less clear that we are
returning NULL for src == NULL case, plus it's an extra indentation level.
For the rest of the patches concerning unescapestr() -- do you really
fix anything
or it's just refactoring? If it's a pure refactoring, there should be a
good reason for it,
which I missed from your comments.
> -
> - for (p1 = p2 = src; *p2; ++p2) {
> - if (*p2 == '\\' && !fl) {
> - fl = 1;
> - } else {
> - *p1++ = *p2;
> - fl = 0;
> + if (src != NULL) {
> + for (p1 = p2 = src; *p2; ++p2) {
> + if (*p2 == '\\' && !fl) {
> + fl = 1;
> + } else {
> + *p1++ = *p2;
> + fl = 0;
> + }
> }
> + *p1 = 0;
> }
> - *p1 = 0;
>
> return src;
> }
More information about the Devel
mailing list