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 <n...@google.com <mailto:n...@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-le...@tycho.nsa.gov
    <mailto:selinux-le...@tycho.nsa.gov>.
    To get help, send an email containing "help" to
    selinux-requ...@tycho.nsa.gov <mailto:selinux-requ...@tycho.nsa.gov>.



_______________________________________________
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


_______________________________________________
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.

Reply via email to