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