[Open-scap] Fixed broken build issues on solaris.
Martin Preisler
mpreisle at redhat.com
Mon Apr 14 13:00:48 UTC 2014
Hi Jacob,
----- Original Message -----
> From: "Simon Lukasik" <slukasik at redhat.com>
> To: "Jacob Varughese" <jacob.varughese at oracle.com>, "Martin Preisler" <mpreisle at redhat.com>
> Cc: open-scap-list at redhat.com
> Sent: Monday, April 14, 2014 11:56:12 AM
> Subject: Re: [Open-scap] Fixed broken build issues on solaris.
>
> On 04/11/2014 08:49 PM, Jacob Varughese wrote:
> > Hi All,
> > Submitting patch to fix broken build issues on solaris.
> >
> > Modified files:
> > src/OVAL/probes/probe/signal_handler.c
> > src/OVAL/probes/unix/solaris/isainfo.c
> > src/SCE/sce_engine.c
> > src/XCCDF/tailoring.c
> > src/XCCDF_POLICY/xccdf_policy.c
> > utils/oscap-info.c
> >
> > Please provide feedback.
> >
>
> Hello Jacob,
>
> Thank You for your contribution! I have pushed some of your changes.
> Then I have decided to amend other stuff (making a different solution).
> And two of the snippets I didn't like. Next time, could you please try
> to send separate patches for each issue?
>
> Bellow I explain each of the change.
>
> Overall, thanks for the patch!
>
> > Thank you,
> > Jacob.
> >
> > 0001-Fixed-broken-build-issues-on-solaris.patch
> >
> >
> >>From eacebd3ad28b12462591692396a81a34f4b53db2 Mon Sep 17 00:00:00 2001
> > From: Jacob Varughese<jacob.varughese at oracle.com>
> > Date: Fri, 11 Apr 2014 11:42:23 -0700
> > Subject: [PATCH] Fixed broken build issues on solaris
> >
> > ---
> > src/OVAL/probes/probe/signal_handler.c | 2 +-
> > src/OVAL/probes/unix/solaris/isainfo.c | 7 ++++++-
> > src/SCE/sce_engine.c | 29
> > +++++++++++++++++++++++++++++
> > src/XCCDF/tailoring.c | 8 ++++++++
> > src/XCCDF_POLICY/xccdf_policy.c | 16 ++++++++++++++++
> > utils/oscap-info.c | 4 ++++
> > 6 files changed, 64 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/OVAL/probes/probe/signal_handler.c
> > b/src/OVAL/probes/probe/signal_handler.c
> > index 0726349..c05b61f 100644
> > --- a/src/OVAL/probes/probe/signal_handler.c
> > +++ b/src/OVAL/probes/probe/signal_handler.c
> > @@ -125,9 +125,9 @@ void *probe_signal_handler(void *arg)
> > * cancelability), but at most 60 seconds per thread.
> > */
> > for (; coll.cnt > 0; --coll.cnt) {
> > + probe_worker_t *thr = coll.thr[coll.cnt - 1];
> > #if defined(HAVE_PTHREAD_TIMEDJOIN_NP) && defined(HAVE_CLOCK_GETTIME)
> > struct timespec j_tm;
> > - probe_worker_t *thr = coll.thr[coll.cnt - 1];
> >
>
> ACK. pushed
>
> > if (clock_gettime(CLOCK_REALTIME, &j_tm) == -1) {
> > dE("clock_gettime(CLOCK_REALTIME): %d, %s.\n", errno,
> > strerror(errno));
> > diff --git a/src/OVAL/probes/unix/solaris/isainfo.c
> > b/src/OVAL/probes/unix/solaris/isainfo.c
> > index 9c950f3..93443c6 100644
> > --- a/src/OVAL/probes/unix/solaris/isainfo.c
> > +++ b/src/OVAL/probes/unix/solaris/isainfo.c
> > @@ -47,6 +47,7 @@
> > #include <arpa/inet.h>
> > #include <regex.h>
> > #include <sys/systeminfo.h>
> > +#include "../../../../common/debug_priv.h"
> >
>
> NACK. I failed to see, why this is needed. Could you please recheck?
>
>
> > /* man sysinfo (2) recommends using 257 for this size */
> > #define MAX_STR_RESULT 257
> > @@ -92,8 +93,12 @@ int read_sysinfo(probe_ctx *ctx) {
> > if (sysinfo(SI_ARCHITECTURE_K, result.kernel_isa, MAX_STR_RESULT) == -1)
> > {
> > return err;
> > }
> > -
> > +#if defined(__SVR4) && defined(__sun)
> > + if ((sysinfo(SI_ARCHITECTURE_32, result.application_isa, MAX_STR_RESULT)
> > == -1) &&
> > + (sysinfo(SI_ARCHITECTURE_64, result.application_isa, MAX_STR_RESULT)
> > == -1)) {
> > +#else
> > if (sysinfo(SI_ARCHITECTURE_NATIVE, result.application_isa,
> > MAX_STR_RESULT) == -1) {
> > +#endif
> > return err;
>
> ACK. pushed.
>
> > }
> >
> > diff --git a/src/SCE/sce_engine.c b/src/SCE/sce_engine.c
> > index 89fe180..ffab708 100644
> > --- a/src/SCE/sce_engine.c
> > +++ b/src/SCE/sce_engine.c
> > @@ -45,7 +45,9 @@
> > #include <assert.h>
> > #include <fcntl.h>
> > #include <sys/types.h>
> > +#if defined(__linux__)
> > #include <sys/prctl.h>
> > +#endif
>
> NACK.
>
> prctl should be present also on platforms than just linux. I would much
> rather like to see an #ifdef(HAS_PRCTL) with appropriate query in
> configure script. What do you think?
>
> > #include <limits.h>
> > #include <unistd.h>
> >
> > @@ -53,7 +55,11 @@ struct sce_check_result
> > {
> > char* href;
> > char* basename;
> > +#if defined(__SVR4) && defined(__sun)
> > + char* std_out;
> > +#else
> > char* stdout;
> > +#endif
> > int exit_code;
> > struct oscap_stringlist* environment_variables;
> > xccdf_test_result_type_t xccdf_result;
> > @@ -64,7 +70,11 @@ struct sce_check_result* sce_check_result_new(void)
> > struct sce_check_result* ret = oscap_alloc(sizeof(struct
> > sce_check_result));
> > ret->href = NULL;
> > ret->basename = NULL;
> > +#if defined(__SVR4) && defined(__sun)
> > + ret->std_out = NULL;
> > +#else
> > ret->stdout = NULL;
> > +#endif
> > ret->environment_variables = oscap_stringlist_new();
> > ret->xccdf_result = XCCDF_RESULT_UNKNOWN;
> >
> > @@ -80,8 +90,13 @@ void sce_check_result_free(struct sce_check_result* v)
> > oscap_free(v->href);
> > if (v->basename)
> > oscap_free(v->basename);
> > +#if defined(__SVR4) && defined(__sun)
> > + if (v->std_out)
> > + oscap_free(v->std_out);
> > +#else
> > if (v->stdout)
> > oscap_free(v->stdout);
> > +#endif
> >
> > oscap_stringlist_free(v->environment_variables);
> >
> > @@ -116,15 +131,25 @@ const char* sce_check_result_get_basename(struct
> > sce_check_result* v)
> >
> > void sce_check_result_set_stdout(struct sce_check_result* v, const char*
> > _stdout)
> > {
> > +#if defined(__SVR4) && defined(__sun)
> > + if (v->std_out)
> > + oscap_free(v->std_out);
> > + v->std_out = strdup(_stdout);
> > +#else
> > if (v->stdout)
> > oscap_free(v->stdout);
> >
> > v->stdout = strdup(_stdout);
> > +#endif
> > }
> >
> > const char* sce_check_result_get_stdout(struct sce_check_result* v)
> > {
> > +#if defined(__SVR4) && defined(__sun)
> > + return v->std_out;
> > +#else
> > return v->stdout;
> > +#endif
> > }
> >
> > void sce_check_result_set_exit_code(struct sce_check_result* v, int
> > exit_code)
> > @@ -179,7 +204,11 @@ void sce_check_result_export(struct sce_check_result*
> > v, const char* target_file
> > oscap_string_iterator_free(it);
> > fprintf(f, "\t</sceres:environment>\n");
> > fprintf(f, "\t<sceres:stdout><![CDATA[\n");
> > +#if defined(__SVR4) && defined(__sun)
> > + fwrite(v->std_out, 1, strlen(v->std_out), f);
> > +#else
> > fwrite(v->stdout, 1, strlen(v->stdout), f);
> > +#endif
>
> I have rewritten the code to use std_out. Hence no need for ifdef.
>
> > fprintf(f, "\t]]></sceres:stdout>\n");
> > fprintf(f, "\t<sceres:exit_code>%i</sceres:exit_code>\n",
> > sce_check_result_get_exit_code(v));
> > fprintf(f, "\t<sceres:result>%s</sceres:result>\n",
> > xccdf_test_result_type_get_text(sce_check_result_get_xccdf_result(v)));
> > diff --git a/src/XCCDF/tailoring.c b/src/XCCDF/tailoring.c
> > index 4c0eb7f..bc2411e 100644
> > --- a/src/XCCDF/tailoring.c
> > +++ b/src/XCCDF/tailoring.c
> > @@ -233,7 +233,11 @@ xmlNodePtr xccdf_tailoring_to_dom(struct
> > xccdf_tailoring *tailoring, xmlDocPtr d
> > xmlNode *tailoring_node = xmlNewNode(ns_xccdf, BAD_CAST "Tailoring");
> >
> > const char *xccdf_version =
> > xccdf_version_info_get_version(version_info);
> > +#ifdef __USE_GNU
> > if (strverscmp(xccdf_version, "1.1") >= 0 && strverscmp(xccdf_version,
> > "1.2") < 0) {
> > +#else
> > + if (strcmp(xccdf_version, "1.1") >= 0 && strcmp(xccdf_version, "1.2") <
> > 0) {
> > +#endif
>
> ACK. pushed
>
> > // XCCDF 1.1 does not support Tailoring!
> > // However we will allow Tailoring export if it is done to an external
> > // file. The namespace will be our custom xccdf-1.1-tailoring extension
> > @@ -250,7 +254,11 @@ xmlNodePtr xccdf_tailoring_to_dom(struct
> > xccdf_tailoring *tailoring, xmlDocPtr d
> > BAD_CAST "cdf-11-tailoring"
> > );
> > }
> > +#ifdef __USE_GNU
> > else if (strverscmp(xccdf_version, "1.1") < 0) {
> > +#else
> > + else if (strcmp(xccdf_version, "1.1") < 0) {
> > +#endif
>
> ACK. pushed
>
> > oscap_seterr(OSCAP_EFAMILY_XML, "XCCDF Tailoring isn't supported in
> > XCCDF version '%s',"
> > "nor does openscap have a custom extension for this scenario. "
> > "XCCDF Tailoring requires XCCDF 1.1 and higher, 1.2 is recommended.");
> > diff --git a/src/XCCDF_POLICY/xccdf_policy.c
> > b/src/XCCDF_POLICY/xccdf_policy.c
> > index 186272a..c1ab12d 100644
> > --- a/src/XCCDF_POLICY/xccdf_policy.c
> > +++ b/src/XCCDF_POLICY/xccdf_policy.c
> > @@ -510,7 +510,11 @@ static struct oscap_list *
> > xccdf_policy_check_get_value_bindings(struct xccdf_po
> > if (r_value != NULL) {
> > selector = xccdf_refine_value_get_selector(r_value);
> > /* This refine value changes the value content */
> > +#if defined(__SVR4) && defined(__sun)
> > + if (((unsigned int)xccdf_refine_value_get_oper(r_value)) >
> > 0) {
> > +#else
> > if (xccdf_refine_value_get_oper(r_value) > 0) {
> > +#endif
> > binding->operator =
> > xccdf_refine_value_get_oper(r_value);
> > } else binding->operator = xccdf_value_get_oper(value);
> >
> > @@ -2165,9 +2169,17 @@ bool xccdf_policy_resolve(struct xccdf_policy *
> > policy)
> >
> > } else if (xccdf_item_get_type(item) == XCCDF_RULE) {
> > /* Perform all changes in rule */
> > +#if defined(__SVR4) && defined(__sun)
> > + if (((unsigned int)xccdf_refine_rule_get_role(r_rule)) >
> > 0)
> > +#else
> > if (xccdf_refine_rule_get_role(r_rule) > 0)
> > +#endif
> > xccdf_rule_set_role((struct xccdf_rule *) item,
> > xccdf_refine_rule_get_role(r_rule));
> > +#if defined(__SVR4) && defined(__sun)
> > + if (!(unsigned int)xccdf_refine_rule_get_severity(r_rule)
> > > 0)
> > +#else
> > if (!xccdf_refine_rule_get_severity(r_rule) > 0)
> > +#endif
> > xccdf_rule_set_severity((struct xccdf_rule *) item,
> > xccdf_refine_rule_get_severity(r_rule));
> >
> > } else {}/* TODO oscap_err ? */;
> > @@ -2454,7 +2466,11 @@ struct xccdf_item * xccdf_policy_tailor_item(struct
> > xccdf_policy * policy, struc
> > if (r_rule == NULL) return item;
> >
> > new_item = (struct xccdf_item *) xccdf_rule_clone((struct
> > xccdf_rule *) item);
> > +#if defined(__SVR4) && defined(__sun)
> > + if (((unsigned int)xccdf_refine_rule_get_role(r_rule)) > 0)
> > +#else
> > if (xccdf_refine_rule_get_role(r_rule) > 0)
> > +#endif
>
> I would like Martin to review these four changes.
The casts make sense because we are comparing an enum to an int. However I don't like that they are conditional. They unnecessarily branch the code out, making maintenance more expensive. I got rid of the #ifdefs and I cast to (int) instead of (unsigned int) because in ANSI C enums are ints. Pushed as a8f64b32902b8d1ba7c90eef27b9d258c7a0ae70
>
> > xccdf_rule_set_role((struct xccdf_rule *) new_item,
> > xccdf_refine_rule_get_role(r_rule));
> > if (xccdf_refine_rule_get_severity(r_rule) > 0)
> > xccdf_rule_set_severity((struct xccdf_rule *) new_item,
> > xccdf_refine_rule_get_severity(r_rule));
> > diff --git a/utils/oscap-info.c b/utils/oscap-info.c
> > index aa0850b..8428061 100644
> > --- a/utils/oscap-info.c
> > +++ b/utils/oscap-info.c
> > @@ -34,7 +34,11 @@
> > #include <time.h>
> > #include <sys/types.h>
> > #include <sys/stat.h>
> > +#if defined(__SVR4) && defined(__sun)
> > +#include <limits.h>
> > +#else
> > #include <linux/limits.h>
> > +#endif
> >
>
> I have changed the include to limits.h everywhere. No need for ifdef then!
>
> > #include <oscap.h>
> > #include <xccdf_policy.h>
> > -- 1.7.9.2
> >
> >
> >
> > _______________________________________________
> > Open-scap-list mailing list
> > Open-scap-list at redhat.com
> > https://www.redhat.com/mailman/listinfo/open-scap-list
> >
>
>
> --
> Simon Lukasik
> Security Technologies, Red Hat, Inc.
>
More information about the Open-scap-list
mailing list