| Message ID | 1382058681-14957-6-git-send-email-xiawenc@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
Il 18/10/2013 03:11, Wenchao Xia ha scritto: > Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com> > --- > include/qapi/error.h | 5 ++++- > qobject/qerror.c | 7 ------- > util/error.c | 6 ------ > 3 files changed, 4 insertions(+), 14 deletions(-) > > diff --git a/include/qapi/error.h b/include/qapi/error.h > index 7d4c696..8688aaf 100644 > --- a/include/qapi/error.h > +++ b/include/qapi/error.h > @@ -20,7 +20,10 @@ > * A class representing internal errors within QEMU. An error has a ErrorClass > * code and a human message. > */ > -typedef struct Error Error; > +typedef struct Error { > + char *msg; > + ErrorClass err_class; > +} Error; Please add a comment that it should be treated as an opaque type. Paolo > > /** > * Set an indirect pointer to an error given a ErrorClass value and a > diff --git a/qobject/qerror.c b/qobject/qerror.c > index 3aee1cf..5b487f3 100644 > --- a/qobject/qerror.c > +++ b/qobject/qerror.c > @@ -97,13 +97,6 @@ void qerror_report(ErrorClass eclass, const char *fmt, ...) > } > } > > -/* Evil... */ > -struct Error > -{ > - char *msg; > - ErrorClass err_class; > -}; > - > void qerror_report_err(Error *err) > { > QError *qerr; > diff --git a/util/error.c b/util/error.c > index ec0faa6..da0d221 100644 > --- a/util/error.c > +++ b/util/error.c > @@ -17,12 +17,6 @@ > #include "qapi-types.h" > #include "qapi/qmp/qerror.h" > > -struct Error > -{ > - char *msg; > - ErrorClass err_class; > -}; > - > void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...) > { > Error *err; >
Paolo Bonzini <pbonzini@redhat.com> writes: > Il 18/10/2013 03:11, Wenchao Xia ha scritto: >> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com> >> --- >> include/qapi/error.h | 5 ++++- >> qobject/qerror.c | 7 ------- >> util/error.c | 6 ------ >> 3 files changed, 4 insertions(+), 14 deletions(-) >> >> diff --git a/include/qapi/error.h b/include/qapi/error.h >> index 7d4c696..8688aaf 100644 >> --- a/include/qapi/error.h >> +++ b/include/qapi/error.h >> @@ -20,7 +20,10 @@ >> * A class representing internal errors within QEMU. An error has a ErrorClass >> * code and a human message. >> */ >> -typedef struct Error Error; >> +typedef struct Error { >> + char *msg; >> + ErrorClass err_class; >> +} Error; > > Please add a comment that it should be treated as an opaque type. Or keep it opaque here, and complete the type in an internal header. But see below. > > Paolo > >> >> /** >> * Set an indirect pointer to an error given a ErrorClass value and a >> diff --git a/qobject/qerror.c b/qobject/qerror.c >> index 3aee1cf..5b487f3 100644 >> --- a/qobject/qerror.c >> +++ b/qobject/qerror.c >> @@ -97,13 +97,6 @@ void qerror_report(ErrorClass eclass, const char *fmt, ...) >> } >> } >> >> -/* Evil... */ >> -struct Error >> -{ >> - char *msg; >> - ErrorClass err_class; >> -}; >> - >> void qerror_report_err(Error *err) >> { >> QError *qerr; qerr = qerror_new(); loc_save(&qerr->loc); qerr->err_msg = g_strdup(err->msg); qerr->err_class = err->err_class; if (monitor_cur_is_qmp()) { monitor_set_error(cur_mon, qerr); } else { qerror_print(qerr); QDECREF(qerr); } } This is the only use of the "evil" duplicate. I suspect it could be cleaned up like this: qerr->err_msg = g_strdup(error_get_pretty(err)); qerr->err_class = error_get_class(err); If that's true, the duplicate goes away, and we can keep the type opaque. >> diff --git a/util/error.c b/util/error.c >> index ec0faa6..da0d221 100644 >> --- a/util/error.c >> +++ b/util/error.c >> @@ -17,12 +17,6 @@ >> #include "qapi-types.h" >> #include "qapi/qmp/qerror.h" >> >> -struct Error >> -{ >> - char *msg; >> - ErrorClass err_class; >> -}; >> - >> void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...) >> { >> Error *err; >>
δΊ 2013/10/18 19:22, Markus Armbruster ει: > Paolo Bonzini <pbonzini@redhat.com> writes: > >> Il 18/10/2013 03:11, Wenchao Xia ha scritto: >>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com> >>> --- >>> include/qapi/error.h | 5 ++++- >>> qobject/qerror.c | 7 ------- >>> util/error.c | 6 ------ >>> 3 files changed, 4 insertions(+), 14 deletions(-) >>> >>> diff --git a/include/qapi/error.h b/include/qapi/error.h >>> index 7d4c696..8688aaf 100644 >>> --- a/include/qapi/error.h >>> +++ b/include/qapi/error.h >>> @@ -20,7 +20,10 @@ >>> * A class representing internal errors within QEMU. An error has a ErrorClass >>> * code and a human message. >>> */ >>> -typedef struct Error Error; >>> +typedef struct Error { >>> + char *msg; >>> + ErrorClass err_class; >>> +} Error; >> Please add a comment that it should be treated as an opaque type. > Or keep it opaque here, and complete the type in an internal header. > But see below. > >> Paolo >> >>> >>> /** >>> * Set an indirect pointer to an error given a ErrorClass value and a >>> diff --git a/qobject/qerror.c b/qobject/qerror.c >>> index 3aee1cf..5b487f3 100644 >>> --- a/qobject/qerror.c >>> +++ b/qobject/qerror.c >>> @@ -97,13 +97,6 @@ void qerror_report(ErrorClass eclass, const char *fmt, ...) >>> } >>> } >>> >>> -/* Evil... */ >>> -struct Error >>> -{ >>> - char *msg; >>> - ErrorClass err_class; >>> -}; >>> - >>> void qerror_report_err(Error *err) >>> { >>> QError *qerr; > qerr = qerror_new(); > loc_save(&qerr->loc); > qerr->err_msg = g_strdup(err->msg); > qerr->err_class = err->err_class; > > if (monitor_cur_is_qmp()) { > monitor_set_error(cur_mon, qerr); > } else { > qerror_print(qerr); > QDECREF(qerr); > } > } > > This is the only use of the "evil" duplicate. I suspect it could be > cleaned up like this: > > qerr->err_msg = g_strdup(error_get_pretty(err)); > qerr->err_class = error_get_class(err); > > If that's true, the duplicate goes away, and we can keep the type > opaque. seems a smart idea, will use it. >>> diff --git a/util/error.c b/util/error.c >>> index ec0faa6..da0d221 100644 >>> --- a/util/error.c >>> +++ b/util/error.c >>> @@ -17,12 +17,6 @@ >>> #include "qapi-types.h" >>> #include "qapi/qmp/qerror.h" >>> >>> -struct Error >>> -{ >>> - char *msg; >>> - ErrorClass err_class; >>> -}; >>> - >>> void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...) >>> { >>> Error *err; >>>
diff --git a/include/qapi/error.h b/include/qapi/error.h index 7d4c696..8688aaf 100644 --- a/include/qapi/error.h +++ b/include/qapi/error.h @@ -20,7 +20,10 @@ * A class representing internal errors within QEMU. An error has a ErrorClass * code and a human message. */ -typedef struct Error Error; +typedef struct Error { + char *msg; + ErrorClass err_class; +} Error; /** * Set an indirect pointer to an error given a ErrorClass value and a diff --git a/qobject/qerror.c b/qobject/qerror.c index 3aee1cf..5b487f3 100644 --- a/qobject/qerror.c +++ b/qobject/qerror.c @@ -97,13 +97,6 @@ void qerror_report(ErrorClass eclass, const char *fmt, ...) } } -/* Evil... */ -struct Error -{ - char *msg; - ErrorClass err_class; -}; - void qerror_report_err(Error *err) { QError *qerr; diff --git a/util/error.c b/util/error.c index ec0faa6..da0d221 100644 --- a/util/error.c +++ b/util/error.c @@ -17,12 +17,6 @@ #include "qapi-types.h" #include "qapi/qmp/qerror.h" -struct Error -{ - char *msg; - ErrorClass err_class; -}; - void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...) { Error *err;
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com> --- include/qapi/error.h | 5 ++++- qobject/qerror.c | 7 ------- util/error.c | 6 ------ 3 files changed, 4 insertions(+), 14 deletions(-)