[Bug-wget] Replace some loops with string.h functions

[Kapus, Timotej]

Hi,

I'm writing a program analysis that tries to find and replace some loops with str* functions. I'm trying to see if these replacements are a useful refactoring or the loops are intentional. I noticed that wget has a couple of these loops and wrote a patch (pasted below) that changes this.

I replaced 1 loop with strspn, which is clearer to me and 1 loop with strrchr, which is both clearer and should probably be faster on most systems. There is also some precedent for these refactorings in commits 1fa77ff5b and e976d4

Cheers,
Timotej

From 0bec668e02073738fe560b92fb2a51712420cf26 Mon Sep 17 00:00:00 2001
From: Timotej Kapus <***@ic.ac.uk>
Date: Fri, 26 Oct 2018 14:28:01 +0100
Subject: [PATCH] Replace loops with string.h functions

* src/init.c Replace loop with strspn
* src/url.c Replace loop with strrchr
---
src/init.c | 5 +++--
src/url.c | 5 ++---
2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/init.c b/src/init.c
index 2fac8ae..64e19b2 100644
--- a/src/init.c
+++ b/src/init.c
@@ -885,8 +885,10 @@ parse_line (const char *line, char **com, char **val, int *comind)

#if defined(WINDOWS) || defined(MSDOS)
# define ISSEP(c) ((c) == '/' || (c) == '\\')
+# define SEPSTRING "/\\"
#else
# define ISSEP(c) ((c) == '/')
+# define SEPSTRING "/"
#endif

/* Run commands[comind].action. */
@@ -927,8 +929,7 @@ setval_internal_tilde (int comind, const char *com, const char *val)
xfree (*pstring);

/* Skip the leading "~/". */
- for (++val; ISSEP (*val); val++)
- ;
+ val += strspn(val + 1, SEPSTRING) + 1;
*pstring = concat_strings (home, "/", val, (char *)0);
xfree (home);
}
diff --git a/src/url.c b/src/url.c
index 2e0b730..4b2263c 100644
--- a/src/url.c
+++ b/src/url.c
@@ -1248,9 +1248,8 @@ mkalldirs (const char *path)
struct stat st;
int res;

- p = path + strlen (path);
- for (; *p != '/' && p != path; p--)
- ;
+ p = strrchr(path, '/');
+ p = p == NULL ? path : p;

/* Don't create if it's just a file. */
if ((p == path) && (*p != '/'))
--
2.7.4

[Tim Rühsen]

Pushed, thanks for your contribution !

Post by Kapus, Timotej
Hi,
I'm writing a program analysis that tries to find and replace some loops with str* functions. I'm trying to see if these replacements are a useful refactoring or the loops are intentional. I noticed that wget has a couple of these loops and wrote a patch (pasted below) that changes this.
I replaced 1 loop with strspn, which is clearer to me and 1 loop with strrchr, which is both clearer and should probably be faster on most systems. There is also some precedent for these refactorings in commits 1fa77ff5b and e976d4
Cheers,
Timotej
From 0bec668e02073738fe560b92fb2a51712420cf26 Mon Sep 17 00:00:00 2001
Date: Fri, 26 Oct 2018 14:28:01 +0100
Subject: [PATCH] Replace loops with string.h functions
* src/init.c Replace loop with strspn
* src/url.c Replace loop with strrchr
---
src/init.c | 5 +++--
src/url.c | 5 ++---
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/init.c b/src/init.c
index 2fac8ae..64e19b2 100644
--- a/src/init.c
+++ b/src/init.c
@@ -885,8 +885,10 @@ parse_line (const char *line, char **com, char **val, int *comind)
#if defined(WINDOWS) || defined(MSDOS)
# define ISSEP(c) ((c) == '/' || (c) == '\\')
+# define SEPSTRING "/\\"
#else
# define ISSEP(c) ((c) == '/')
+# define SEPSTRING "/"
#endif
/* Run commands[comind].action. */
@@ -927,8 +929,7 @@ setval_internal_tilde (int comind, const char *com, const char *val)
xfree (*pstring);
/* Skip the leading "~/". */
- for (++val; ISSEP (*val); val++)
- ;
+ val += strspn(val + 1, SEPSTRING) + 1;
*pstring = concat_strings (home, "/", val, (char *)0);
xfree (home);
}
diff --git a/src/url.c b/src/url.c
index 2e0b730..4b2263c 100644
--- a/src/url.c
+++ b/src/url.c
@@ -1248,9 +1248,8 @@ mkalldirs (const char *path)
struct stat st;
int res;
- p = path + strlen (path);
- for (; *p != '/' && p != path; p--)
- ;
+ p = strrchr(path, '/');
+ p = p == NULL ? path : p;
/* Don't create if it's just a file. */
if ((p == path) && (*p != '/'))
--
2.7.4