Re: make: clarify an error string

2017-07-24 Thread Todd C. Miller
On Mon, 24 Jul 2017 13:02:12 +0200, Marc Espie wrote:

> Here's a proper error message:

OK millert@

 - todd



Re: make: clarify an error string

2017-07-24 Thread Ingo Schwarze
Hi Marc,

Marc Espie wrote on Mon, Jul 24, 2017 at 01:02:12PM +0200:
> On Mon, Jul 24, 2017 at 10:43:03AM +0200, Marc Espie wrote:
>> On Mon, Jul 24, 2017 at 03:15:32PM +0800, Michael W. Bombardieri wrote:

>>> In make(1), do_run_command() has a sanity check for a null command string.
>>> The error message passes the null string to printf(). Is this any better?

> Here's a proper error message:

Works for me, looks good to code inspection, and i don't see how it
could possibly break anything, so OK schwarze@.

Yours,
  Ingo


> Index: engine.c
> ===
> RCS file: /cvs/src/usr.bin/make/engine.c,v
> retrieving revision 1.53
> diff -u -p -r1.53 engine.c
> --- engine.c  9 Jul 2017 15:28:00 -   1.53
> +++ engine.c  24 Jul 2017 10:56:38 -
> @@ -734,7 +734,7 @@ setup_engine(void)
>  }
>  
>  static bool
> -do_run_command(Job *job)
> +do_run_command(Job *job, const char *pre)
>  {
>   bool silent;/* Don't print command */
>   bool doExecute; /* Execute the command */
> @@ -752,7 +752,9 @@ do_run_command(Job *job)
>   /* How can we execute a null command ? we warn the user that the
>* command expanded to nothing (is this the right thing to do?).  */
>   if (*cmd == '\0') {
> - Error("%s expands to empty string", cmd);
> + Parse_Error(PARSE_WARNING, 
> + "'%s' expands to '' while building %s", 
> + pre, job->node->name);
>   return false;
>   }
>  
> @@ -833,7 +835,7 @@ job_run_next(Job *job)
>   job->next_cmd = Lst_Adv(job->next_cmd);
>   if (fatal_errors)
>   Punt(NULL);
> - started = do_run_command(job);
> + started = do_run_command(job, command->string);
>   if (started)
>   return false;
>   else
> 



Re: make: clarify an error string

2017-07-24 Thread Marc Espie
On Mon, Jul 24, 2017 at 10:43:03AM +0200, Marc Espie wrote:
> On Mon, Jul 24, 2017 at 03:15:32PM +0800, Michael W. Bombardieri wrote:
> > Hi,
> > 
> > In make(1), do_run_command() has a sanity check for a null command string.
> > The error message passes the null string to printf(). Is this any better?
> > 
> > - Michael
> >  

Here's a proper error message:

Index: engine.c
===
RCS file: /cvs/src/usr.bin/make/engine.c,v
retrieving revision 1.53
diff -u -p -r1.53 engine.c
--- engine.c9 Jul 2017 15:28:00 -   1.53
+++ engine.c24 Jul 2017 10:56:38 -
@@ -734,7 +734,7 @@ setup_engine(void)
 }
 
 static bool
-do_run_command(Job *job)
+do_run_command(Job *job, const char *pre)
 {
bool silent;/* Don't print command */
bool doExecute; /* Execute the command */
@@ -752,7 +752,9 @@ do_run_command(Job *job)
/* How can we execute a null command ? we warn the user that the
 * command expanded to nothing (is this the right thing to do?).  */
if (*cmd == '\0') {
-   Error("%s expands to empty string", cmd);
+   Parse_Error(PARSE_WARNING, 
+   "'%s' expands to '' while building %s", 
+   pre, job->node->name);
return false;
}
 
@@ -833,7 +835,7 @@ job_run_next(Job *job)
job->next_cmd = Lst_Adv(job->next_cmd);
if (fatal_errors)
Punt(NULL);
-   started = do_run_command(job);
+   started = do_run_command(job, command->string);
if (started)
return false;
else



Re: make: clarify an error string

2017-07-24 Thread Marc Espie
On Mon, Jul 24, 2017 at 03:15:32PM +0800, Michael W. Bombardieri wrote:
> Hi,
> 
> In make(1), do_run_command() has a sanity check for a null command string.
> The error message passes the null string to printf(). Is this any better?
> 
> - Michael
>  
> 
> Index: engine.c
> ===
> RCS file: /cvs/src/usr.bin/make/engine.c,v
> retrieving revision 1.53
> diff -u -p -u -r1.53 engine.c
> --- engine.c  9 Jul 2017 15:28:00 -   1.53
> +++ engine.c  24 Jul 2017 06:58:31 -
> @@ -752,7 +752,7 @@ do_run_command(Job *job)
>   /* How can we execute a null command ? we warn the user that the
>* command expanded to nothing (is this the right thing to do?).  */
>   if (*cmd == '\0') {
> - Error("%s expands to empty string", cmd);
> + Error("command expands to empty string");
>   return false;
>   }
>  
Good catch. I'll do something better, since I know where the command
comes from, so I can display a proper error message with line number,
and (more importantly) the string before substitution.



make: clarify an error string

2017-07-24 Thread Michael W. Bombardieri
Hi,

In make(1), do_run_command() has a sanity check for a null command string.
The error message passes the null string to printf(). Is this any better?

- Michael
 

Index: engine.c
===
RCS file: /cvs/src/usr.bin/make/engine.c,v
retrieving revision 1.53
diff -u -p -u -r1.53 engine.c
--- engine.c9 Jul 2017 15:28:00 -   1.53
+++ engine.c24 Jul 2017 06:58:31 -
@@ -752,7 +752,7 @@ do_run_command(Job *job)
/* How can we execute a null command ? we warn the user that the
 * command expanded to nothing (is this the right thing to do?).  */
if (*cmd == '\0') {
-   Error("%s expands to empty string", cmd);
+   Error("command expands to empty string");
return false;
}