On Wed, Sep 19, 2018 at 12:36 PM Stephen Smalley <[email protected]> wrote:
> On 09/19/2018 03:21 PM, William Roberts wrote: > > Some people might be checking this output since it's been there so long, > > -s would be a good way to go. > > > > Alternatively, a way to bring back this information via a verbose option > > -V could > > be considered. > > > > Either way, a simple logging mechanism analogous to > > LOGV/LOGW/LOGE could be useful, I wonder what subordinate routines > > are logging. IIRC it was all fprintf(stderr) stuff in libselinux proper > > as you allude > > to in the redirection of stdout comment. We don't need to address this > > in this > > patch, but we may want to consider it at some point. > > > > I would lean towards a silent flag as it's backwards compatible, > > but noting that it doesn't suppress subordinate callers. > > > > I would also yield that opinion, as removing it works for me. > > I'm ok dropping the output unless someone knows of an existing user that > relies upon it (which I can't really envision). > Why don't we extend the review period to 72 hours, and ill apply this Friday unless we hear of this breaking someone. Essentially consider this a soft-ack. > > With regard to subordinate routines, libsepol has sepol_debug(0) or > sepol_msg_set_callback() to suppress or redirect its logging. > checkpolicy doesn't use libselinux but it likewise has > selinux_set_callback(). > What about things like: libselinux/src/load_policy.c:299: fprintf(stderr, "libselinux: %s\n", errormsg); Also utils and others are using fprintf directly.... perhaps something we wish to make common across utilities and subordinate libs. > > > > On Wed, Sep 19, 2018 at 12:13 PM Nick Kralevich via Selinux > > <[email protected] <mailto:[email protected]>> wrote: > > > > Reduce noise when calling the checkpolicy command line. In Android, > this > > creates unnecessary build noise which we'd like to avoid. > > > > https://en.wikipedia.org/wiki/Unix_philosophy > > > > Rule of Silence > > Developers should design programs so that they do not print > > unnecessary output. This rule aims to allow other programs > > and developers to pick out the information they need from a > > program's output without having to parse verbosity. > > > > An alternative approach would be to add a -s (silent) option to these > > tools, or to have the Android build system redirect stdout to > /dev/null. > > > > Signed-off-by: Nick Kralevich <[email protected] <mailto:[email protected] > >> > > --- > > checkpolicy/checkmodule.c | 8 -------- > > checkpolicy/checkpolicy.c | 11 ----------- > > 2 files changed, 19 deletions(-) > > > > diff --git a/checkpolicy/checkmodule.c b/checkpolicy/checkmodule.c > > index 46ce258f..8edc1f8c 100644 > > --- a/checkpolicy/checkmodule.c > > +++ b/checkpolicy/checkmodule.c > > @@ -228,7 +228,6 @@ int main(int argc, char **argv) > > if (optind != argc) > > usage(argv[0]); > > } > > - printf("%s: loading policy configuration from %s\n", > > argv[0], file); > > > > /* Set policydb and sidtab used by libsepol service > functions > > to my structures, so that I can directly populate and > > @@ -302,8 +301,6 @@ int main(int argc, char **argv) > > > > sepol_sidtab_destroy(&sidtab); > > > > - printf("%s: policy configuration loaded\n", argv[0]); > > - > > if (outfile) { > > FILE *outfp = fopen(outfile, "w"); > > > > @@ -313,16 +310,11 @@ int main(int argc, char **argv) > > } > > > > if (!cil) { > > - printf("%s: writing binary representation > > (version %d) to %s\n", > > - argv[0], policyvers, outfile); > > - > > if (write_binary_policy(&modpolicydb, > > outfp) != 0) { > > fprintf(stderr, "%s: error writing > > %s\n", argv[0], outfile); > > exit(1); > > } > > } else { > > - printf("%s: writing CIL to %s\n",argv[0], > > outfile); > > - > > if (sepol_module_policydb_to_cil(outfp, > > &modpolicydb, 0) != 0) { > > fprintf(stderr, "%s: error writing > > %s\n", argv[0], outfile); > > exit(1); > > diff --git a/checkpolicy/checkpolicy.c b/checkpolicy/checkpolicy.c > > index fbda4558..12c4c405 100644 > > --- a/checkpolicy/checkpolicy.c > > +++ b/checkpolicy/checkpolicy.c > > @@ -512,8 +512,6 @@ int main(int argc, char **argv) > > if (optind != argc) > > usage(argv[0]); > > } > > - printf("%s: loading policy configuration from %s\n", > > argv[0], file); > > - > > /* Set policydb and sidtab used by libsepol service > functions > > to my structures, so that I can directly populate and > > manipulate them. */ > > @@ -623,8 +621,6 @@ int main(int argc, char **argv) > > if (policydb_load_isids(&policydb, &sidtab)) > > exit(1); > > > > - printf("%s: policy configuration loaded\n", argv[0]); > > - > > if (outfile) { > > outfp = fopen(outfile, "w"); > > if (!outfp) { > > @@ -636,8 +632,6 @@ int main(int argc, char **argv) > > > > if (!cil) { > > if (!conf) { > > - printf("%s: writing binary > > representation (version %d) to %s\n", argv[0], policyvers, outfile); > > - > > policydb.policy_type = POLICY_KERN; > > > > policy_file_init(&pf); > > @@ -645,8 +639,6 @@ int main(int argc, char **argv) > > pf.fp = outfp; > > ret = policydb_write(&policydb, > &pf); > > } else { > > - printf("%s: writing policy.conf to > > %s\n", > > - argv[0], outfile); > > ret = > > sepol_kernel_policydb_to_conf(outfp, policydbp); > > } > > if (ret) { > > @@ -655,7 +647,6 @@ int main(int argc, char **argv) > > exit(1); > > } > > } else { > > - printf("%s: writing CIL to %s\n",argv[0], > > outfile); > > if (binary) { > > ret = > > sepol_kernel_policydb_to_cil(outfp, policydbp); > > } else { > > @@ -894,8 +885,6 @@ int main(int argc, char **argv) > > FGETS(ans, sizeof(ans), stdin); > > pathlen = strlen(ans); > > ans[pathlen - 1] = 0; > > - printf("%s: loading policy configuration > > from %s\n", > > - argv[0], ans); > > fd = open(ans, O_RDONLY); > > if (fd < 0) { > > fprintf(stderr, "Can't open '%s': > > %s\n", > > -- > > 2.19.0.397.gdd90340f6a-goog > > > > _______________________________________________ > > Selinux mailing list > > [email protected] <mailto:[email protected]> > > To unsubscribe, send email to [email protected] > > <mailto:[email protected]>. > > To get help, send an email containing "help" to > > [email protected] <mailto:[email protected] > >. > > > > > > > > _______________________________________________ > > Selinux mailing list > > [email protected] > > To unsubscribe, send email to [email protected]. > > To get help, send an email containing "help" to > [email protected]. > > > >
_______________________________________________ Selinux mailing list [email protected] To unsubscribe, send email to [email protected]. To get help, send an email containing "help" to [email protected].
