[PATCH] livepatch-tools: remove usage of error.h

Roger Pau Monne posted 1 patch 1 year ago
Failed in applying to current master (apply log)
There is a newer version of this series
common.h             | 10 ++++++----
create-diff-object.c |  1 -
lookup.c             |  7 +++++--
prelink.c            |  1 -
4 files changed, 11 insertions(+), 8 deletions(-)
[PATCH] livepatch-tools: remove usage of error.h
Posted by Roger Pau Monne 1 year ago
It's a GNU libc specific header which prevents building on musl for
example.  Instead open code an equivalent replacement for the usage
of ERROR() and DIFF_FATAL() macros.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Ross Lagerwall <ross.lagerwall@citrix.com>
---
 common.h             | 10 ++++++----
 create-diff-object.c |  1 -
 lookup.c             |  7 +++++--
 prelink.c            |  1 -
 4 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/common.h b/common.h
index 9a9da79..ec2ea33 100644
--- a/common.h
+++ b/common.h
@@ -1,18 +1,20 @@
 #ifndef _COMMON_H_
 #define _COMMON_H_
 
-#include <error.h>
-
 extern char *childobj;
 
 #define ERROR(format, ...) \
-	error(1, 0, "ERROR: %s: %s: %d: " format, childobj, __FUNCTION__, __LINE__, ##__VA_ARGS__)
+({ \
+	fflush(stdout); \
+	fprintf(stderr, "ERROR: %s: %s: %d: " format "\n", childobj, __FUNCTION__, __LINE__, ##__VA_ARGS__); \
+	exit(1); \
+})
 
 #define DIFF_FATAL(format, ...) \
 ({ \
 	fflush(stdout); \
 	fprintf(stderr, "ERROR: %s: " format "\n", childobj, ##__VA_ARGS__); \
-	error(2, 0, "unreconcilable difference"); \
+	exit(2); \
 })
 
 #define log_debug(format, ...) log(DEBUG, format, ##__VA_ARGS__)
diff --git a/create-diff-object.c b/create-diff-object.c
index 780e6c8..d8a0032 100644
--- a/create-diff-object.c
+++ b/create-diff-object.c
@@ -45,7 +45,6 @@
 #include <string.h>
 #include <libgen.h>
 #include <argp.h>
-#include <error.h>
 #include <unistd.h>
 #include <time.h>
 #include <gelf.h>
diff --git a/lookup.c b/lookup.c
index 39125c6..b440102 100644
--- a/lookup.c
+++ b/lookup.c
@@ -28,14 +28,17 @@
 #include <stdlib.h>
 #include <stdio.h>
 #include <string.h>
-#include <error.h>
 #include <gelf.h>
 #include <unistd.h>
 
 #include "lookup.h"
 
 #define ERROR(format, ...) \
-	error(1, 0, "%s: %d: " format, __FUNCTION__, __LINE__, ##__VA_ARGS__)
+({ \
+	fflush(stdout); \
+	fprintf(stderr, "%s: %d: " format, __FUNCTION__, __LINE__, ##__VA_ARGS__); \
+	exit(1); \
+})
 
 struct symbol {
 	unsigned long value;
diff --git a/prelink.c b/prelink.c
index 2039e5b..18c5159 100644
--- a/prelink.c
+++ b/prelink.c
@@ -27,7 +27,6 @@
 #include <string.h>
 #include <libgen.h>
 #include <argp.h>
-#include <error.h>
 #include <unistd.h>
 #include <gelf.h>
 
-- 
2.40.0


Re: [PATCH] livepatch-tools: remove usage of error.h
Posted by Andrew Cooper 1 year ago
On 06/04/2023 10:18 am, Roger Pau Monne wrote:
> It's a GNU libc specific header which prevents building on musl for
> example.  Instead open code an equivalent replacement for the usage
> of ERROR() and DIFF_FATAL() macros.
>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Ross Lagerwall <ross.lagerwall@citrix.com>
> ---
>  common.h             | 10 ++++++----
>  create-diff-object.c |  1 -
>  lookup.c             |  7 +++++--
>  prelink.c            |  1 -
>  4 files changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/common.h b/common.h
> index 9a9da79..ec2ea33 100644
> --- a/common.h
> +++ b/common.h
> @@ -1,18 +1,20 @@
>  #ifndef _COMMON_H_
>  #define _COMMON_H_
>  
> -#include <error.h>
> -
>  extern char *childobj;
>  
>  #define ERROR(format, ...) \
> -	error(1, 0, "ERROR: %s: %s: %d: " format, childobj, __FUNCTION__, __LINE__, ##__VA_ARGS__)
> +({ \
> +	fflush(stdout); \
> +	fprintf(stderr, "ERROR: %s: %s: %d: " format "\n", childobj, __FUNCTION__, __LINE__, ##__VA_ARGS__); \
> +	exit(1); \
> +})
>  
>  #define DIFF_FATAL(format, ...) \
>  ({ \
>  	fflush(stdout); \
>  	fprintf(stderr, "ERROR: %s: " format "\n", childobj, ##__VA_ARGS__); \
> -	error(2, 0, "unreconcilable difference"); \
> +	exit(2); \
>  })

Looking at the usage, can't we just use err() instead?

Also, I suspect you don't intend to delete the error message in
DIFF_FATAL() ?

~Andrew

Re: [PATCH] livepatch-tools: remove usage of error.h
Posted by Roger Pau Monné 1 year ago
On Thu, Apr 06, 2023 at 10:36:37AM +0100, Andrew Cooper wrote:
> On 06/04/2023 10:18 am, Roger Pau Monne wrote:
> > It's a GNU libc specific header which prevents building on musl for
> > example.  Instead open code an equivalent replacement for the usage
> > of ERROR() and DIFF_FATAL() macros.
> >
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > ---
> > Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > Cc: Ross Lagerwall <ross.lagerwall@citrix.com>
> > ---
> >  common.h             | 10 ++++++----
> >  create-diff-object.c |  1 -
> >  lookup.c             |  7 +++++--
> >  prelink.c            |  1 -
> >  4 files changed, 11 insertions(+), 8 deletions(-)
> >
> > diff --git a/common.h b/common.h
> > index 9a9da79..ec2ea33 100644
> > --- a/common.h
> > +++ b/common.h
> > @@ -1,18 +1,20 @@
> >  #ifndef _COMMON_H_
> >  #define _COMMON_H_
> >  
> > -#include <error.h>
> > -
> >  extern char *childobj;
> >  
> >  #define ERROR(format, ...) \
> > -	error(1, 0, "ERROR: %s: %s: %d: " format, childobj, __FUNCTION__, __LINE__, ##__VA_ARGS__)
> > +({ \
> > +	fflush(stdout); \
> > +	fprintf(stderr, "ERROR: %s: %s: %d: " format "\n", childobj, __FUNCTION__, __LINE__, ##__VA_ARGS__); \
> > +	exit(1); \
> > +})
> >  
> >  #define DIFF_FATAL(format, ...) \
> >  ({ \
> >  	fflush(stdout); \
> >  	fprintf(stderr, "ERROR: %s: " format "\n", childobj, ##__VA_ARGS__); \
> > -	error(2, 0, "unreconcilable difference"); \
> > +	exit(2); \
> >  })
> 
> Looking at the usage, can't we just use err() instead?

Hm, err() will unconditionaly use errno, which doesn't seem wanted
here, as in both cases errnum is passed as 0, effectively preventing
printing it.

I could use errx(), as that doesn't append an error message, I think
that's available on musl.

Let me know if you agree with that substitution.

> Also, I suspect you don't intend to delete the error message in
> DIFF_FATAL() ?

I didn't think it was that helpful, but I could keep it.

Thanks, Roger.

Re: [PATCH] livepatch-tools: remove usage of error.h
Posted by Andrew Cooper 1 year ago
On 06/04/2023 11:54 am, Roger Pau Monné wrote:
> On Thu, Apr 06, 2023 at 10:36:37AM +0100, Andrew Cooper wrote:
>> On 06/04/2023 10:18 am, Roger Pau Monne wrote:
>>> It's a GNU libc specific header which prevents building on musl for
>>> example.  Instead open code an equivalent replacement for the usage
>>> of ERROR() and DIFF_FATAL() macros.
>>>
>>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>>> ---
>>> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>>> Cc: Ross Lagerwall <ross.lagerwall@citrix.com>
>>> ---
>>>  common.h             | 10 ++++++----
>>>  create-diff-object.c |  1 -
>>>  lookup.c             |  7 +++++--
>>>  prelink.c            |  1 -
>>>  4 files changed, 11 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/common.h b/common.h
>>> index 9a9da79..ec2ea33 100644
>>> --- a/common.h
>>> +++ b/common.h
>>> @@ -1,18 +1,20 @@
>>>  #ifndef _COMMON_H_
>>>  #define _COMMON_H_
>>>  
>>> -#include <error.h>
>>> -
>>>  extern char *childobj;
>>>  
>>>  #define ERROR(format, ...) \
>>> -	error(1, 0, "ERROR: %s: %s: %d: " format, childobj, __FUNCTION__, __LINE__, ##__VA_ARGS__)
>>> +({ \
>>> +	fflush(stdout); \
>>> +	fprintf(stderr, "ERROR: %s: %s: %d: " format "\n", childobj, __FUNCTION__, __LINE__, ##__VA_ARGS__); \
>>> +	exit(1); \
>>> +})
>>>  
>>>  #define DIFF_FATAL(format, ...) \
>>>  ({ \
>>>  	fflush(stdout); \
>>>  	fprintf(stderr, "ERROR: %s: " format "\n", childobj, ##__VA_ARGS__); \
>>> -	error(2, 0, "unreconcilable difference"); \
>>> +	exit(2); \
>>>  })
>> Looking at the usage, can't we just use err() instead?
> Hm, err() will unconditionaly use errno, which doesn't seem wanted
> here, as in both cases errnum is passed as 0, effectively preventing
> printing it.
>
> I could use errx(), as that doesn't append an error message, I think
> that's available on musl.
>
> Let me know if you agree with that substitution.

Yeah, anything in err.h ought to be fine.

>
>> Also, I suspect you don't intend to delete the error message in
>> DIFF_FATAL() ?
> I didn't think it was that helpful, but I could keep it.

I'd be hesitant to drop it, considering how much shell parsing there is
of these tools.

But ultimately it's up to Konrad/Ross.

~Andrew