Re: [PATCH util/makedepend V2] Quote colons in filenames/paths

[Alan Coopersmith]

On 05/ 6/18 11:14 AM, Walter Harms wrote:
> 
> 
>> Alan Coopersmith  hat am 5. Mai 2018 um 20:11
>> geschrieben:
>>
>>
>> From: Antonio Larrosa 
>>
>> Makefile doesn't like colons in filenames/paths so they must
>> be quoted in the output. Otherwise makedepend doesn't work with
>> full paths that contain a colon.
>>
>> V2: Use quoted filename when measuring name length
>> Signed-off-by: Alan Coopersmith 
>> ---
>>  pr.c | 48 
>>  1 file changed, 44 insertions(+), 4 deletions(-)
>>
>> diff --git a/pr.c b/pr.c
>> index 04abef9..9a49635 100644
>> --- a/pr.c
>> +++ b/pr.c
>> @@ -63,25 +63,65 @@ add_include(struct filepointer *filep, struct inclist
>> *file,
>>  }
>>  }
>>  
>> +/**
>> + * Replaces all ":" occurrences in @p input with "\:" using @p outputbuffer
>> (of size @p bufsize)
>> + * possibly to hold the result. @p returns the string with quoted colons
>> + */
>> +static const char *
>> +quoteColons(const char *input, char *outputbuffer, size_t bufsize)
>> +{
>> +const char *tmp=input;
>> +const char *loc;
>> +char *output=outputbuffer;
>> +
>> +loc = strchr(input, ':');
>> +if (loc == NULL) {
>> +return input;
>> +}
>> +
>> +tmp=input;
>> +while (loc != NULL && bufsize > loc-tmp+2 ) {
>> +memcpy(output, tmp, loc-tmp);
>> +output+=loc-tmp;
>> +bufsize-=loc-tmp+2;
>> +tmp=loc+1;
>> +*output='\\';
>> +output++;
>> +*output=':';
>> +output++;
>> +loc = strchr(tmp, ':');
>> +}
>> +
>> +if (strlen(tmp) <= bufsize)
>> +   strcpy(output, tmp);
>> +else {
>> +   strncpy(output, tmp, bufsize-1);
>> +   output[bufsize]=0;
>> +}
>> +return outputbuffer;
>> +}
>> +
> hi,
> i am sure that this is a very fast code but ..
> do you not thing it would be better to use a more simple solution ?
> like:
> 
> for( ; *s ; s++) {
> if (*s == ':')
> *d++='\\';
> *d++=*s;
> }
> *d=*s;
> or did i miss a trick here ?

Antonio will have to answer why he did it that way, I left it since
it seemed to work and didn't look like a problem.

> BTW why 
>  fwrite(buf, len, 1, stdout);
> 
> Is there a loop that i am missing ? Otherwise a simple printf()
> would do instead of snprinft()

That was what the existing code has always done, and it wasn't part of
what Antonio had submitted, but I had the same thought, so put it in a
separate patch:

https://patchwork.freedesktop.org/patch/220801/

-- 
-Alan Coopersmith-   alan.coopersm...@oracle.com
 Oracle Solaris Engineering - https://blogs.oracle.com/alanc
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH util/makedepend V2] Quote colons in filenames/paths

[Walter Harms]

> Alan Coopersmith  hat am 5. Mai 2018 um 20:11
> geschrieben:
> 
> 
> From: Antonio Larrosa 
> 
> Makefile doesn't like colons in filenames/paths so they must
> be quoted in the output. Otherwise makedepend doesn't work with
> full paths that contain a colon.
> 
> V2: Use quoted filename when measuring name length
> Signed-off-by: Alan Coopersmith 
> ---
>  pr.c | 48 
>  1 file changed, 44 insertions(+), 4 deletions(-)
> 
> diff --git a/pr.c b/pr.c
> index 04abef9..9a49635 100644
> --- a/pr.c
> +++ b/pr.c
> @@ -63,25 +63,65 @@ add_include(struct filepointer *filep, struct inclist
> *file,
>   }
>  }
>  
> +/**
> + * Replaces all ":" occurrences in @p input with "\:" using @p outputbuffer
> (of size @p bufsize)
> + * possibly to hold the result. @p returns the string with quoted colons
> + */
> +static const char *
> +quoteColons(const char *input, char *outputbuffer, size_t bufsize)
> +{
> +const char *tmp=input;
> +const char *loc;
> +char *output=outputbuffer;
> +
> +loc = strchr(input, ':');
> +if (loc == NULL) {
> +return input;
> +}
> +
> +tmp=input;
> +while (loc != NULL && bufsize > loc-tmp+2 ) {
> +memcpy(output, tmp, loc-tmp);
> +output+=loc-tmp;
> +bufsize-=loc-tmp+2;
> +tmp=loc+1;
> +*output='\\';
> +output++;
> +*output=':';
> +output++;
> +loc = strchr(tmp, ':');
> +}
> +
> +if (strlen(tmp) <= bufsize)
> +   strcpy(output, tmp);
> +else {
> +   strncpy(output, tmp, bufsize-1);
> +   output[bufsize]=0;
> +}
> +return outputbuffer;
> +}
> +
hi,
i am sure that this is a very fast code but ..
do you not thing it would be better to use a more simple solution ?
like:

for( ; *s ; s++) {
if (*s == ':')
*d++='\\';
*d++=*s;
}
*d=*s;
or did i miss a trick here ?

To avoid a const size buffer you could simply add:
char d[strlen(ip->i_file)*2+1];

and return strdup(d);

BTW why 
 fwrite(buf, len, 1, stdout);

Is there a loop that i am missing ? Otherwise a simple printf()
would do instead of snprinft()

just my 2 cents,
re,
 wh

>  static void
>  pr(struct inclist *ip, const char *file, const char *base)
>  {
>   static const char *lastfile;
>   static int  current_len;
>   register intlen, i;
> + const char *quoted;
>   charbuf[ BUFSIZ ];
> + charquotebuf[ BUFSIZ ];
>  
>   printed = TRUE;
> - len = strlen(ip->i_file)+1;
> + quoted = quoteColons(ip->i_file, quotebuf, sizeof(quotebuf));
> + len = strlen(quoted)+1;
>   if (current_len + len > width || file != lastfile) {
>   lastfile = file;
>   snprintf(buf, sizeof(buf), "\n%s%s%s: %s",
> -  objprefix, base, objsuffix, ip->i_file);
> +  objprefix, base, objsuffix, quoted);
>   len = current_len = strlen(buf);
>   }
>   else {
> - buf[0] = ' ';
> - strcpy(buf+1, ip->i_file);
> + snprintf(buf, sizeof(buf), " %s", quoted);
>   current_len += len;
>   }
>   fwrite(buf, len, 1, stdout);
> -- 
> 2.15.0
> 
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: https://lists.x.org/mailman/listinfo/xorg-devel
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH util/makedepend V2] Quote colons in filenames/paths

[Alan Coopersmith]

From: Antonio Larrosa 

Makefile doesn't like colons in filenames/paths so they must
be quoted in the output. Otherwise makedepend doesn't work with
full paths that contain a colon.

V2: Use quoted filename when measuring name length
Signed-off-by: Alan Coopersmith 
---
 pr.c | 48 
 1 file changed, 44 insertions(+), 4 deletions(-)

diff --git a/pr.c b/pr.c
index 04abef9..9a49635 100644
--- a/pr.c
+++ b/pr.c
@@ -63,25 +63,65 @@ add_include(struct filepointer *filep, struct inclist *file,
}
 }
 
+/**
+ * Replaces all ":" occurrences in @p input with "\:" using @p outputbuffer 
(of size @p bufsize)
+ * possibly to hold the result. @p returns the string with quoted colons
+ */
+static const char *
+quoteColons(const char *input, char *outputbuffer, size_t bufsize)
+{
+const char *tmp=input;
+const char *loc;
+char *output=outputbuffer;
+
+loc = strchr(input, ':');
+if (loc == NULL) {
+return input;
+}
+
+tmp=input;
+while (loc != NULL && bufsize > loc-tmp+2 ) {
+memcpy(output, tmp, loc-tmp);
+output+=loc-tmp;
+bufsize-=loc-tmp+2;
+tmp=loc+1;
+*output='\\';
+output++;
+*output=':';
+output++;
+loc = strchr(tmp, ':');
+}
+
+if (strlen(tmp) <= bufsize)
+   strcpy(output, tmp);
+else {
+   strncpy(output, tmp, bufsize-1);
+   output[bufsize]=0;
+}
+return outputbuffer;
+}
+
 static void
 pr(struct inclist *ip, const char *file, const char *base)
 {
static const char *lastfile;
static int  current_len;
register intlen, i;
+   const char *quoted;
charbuf[ BUFSIZ ];
+   charquotebuf[ BUFSIZ ];
 
printed = TRUE;
-   len = strlen(ip->i_file)+1;
+   quoted = quoteColons(ip->i_file, quotebuf, sizeof(quotebuf));
+   len = strlen(quoted)+1;
if (current_len + len > width || file != lastfile) {
lastfile = file;
snprintf(buf, sizeof(buf), "\n%s%s%s: %s",
-objprefix, base, objsuffix, ip->i_file);
+objprefix, base, objsuffix, quoted);
len = current_len = strlen(buf);
}
else {
-   buf[0] = ' ';
-   strcpy(buf+1, ip->i_file);
+   snprintf(buf, sizeof(buf), " %s", quoted);
current_len += len;
}
fwrite(buf, len, 1, stdout);
-- 
2.15.0

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel