From 827fd5b3cb9cda5bb4d934bf54122cada82b859b Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Fri, 26 May 2017 14:05:15 -0400
Subject: [PATCH 27/28] FIXME: fixes

---
 checkers/checker.py |  5 +++++
 checkers/ianal.py   |  1 -
 gcc/checkers.c      | 52 ++++++++++++++++++++++++++++------------------------
 gcc/firehose.c      |  2 +-
 4 files changed, 34 insertions(+), 26 deletions(-)

diff --git a/checkers/checker.py b/checkers/checker.py
index 454da4f..27489ca 100755
--- a/checkers/checker.py
+++ b/checkers/checker.py
@@ -16,6 +16,11 @@
 #   along with this program.  If not, see
 #   <http://www.gnu.org/licenses/>.
 
+"""
+A "checker" is an executable which takes GCC-style command-line
+arguments and writes a Firehose JSON file to stdout.
+"""
+
 import json
 import logging
 import os
diff --git a/checkers/ianal.py b/checkers/ianal.py
index 73081b0..3fd93ea 100755
--- a/checkers/ianal.py
+++ b/checkers/ianal.py
@@ -45,7 +45,6 @@ class NotALawyer(Checker):
                 m = re.match('.*(Copyright).*', line)
                 if m:
                     start, end = m.span(1)
-                    # FIXME: use proper range
                     linenum = lineidx + 1
                     range_ = Range(start=Point(linenum, start + 1),
                                    end=Point(linenum, end))
diff --git a/gcc/checkers.c b/gcc/checkers.c
index 4b213a5a..b2e8ce3 100644
--- a/gcc/checkers.c
+++ b/gcc/checkers.c
@@ -104,7 +104,7 @@ diagnostic_at (location_t loc, diagnostic_info *diagnostic,
   va_end (ap);
 }
 
-/* FIXME.  */
+/* Emit a diagnostic for ISSUE.  */
 
 static void
 handle_issue (const firehose::analysis &analysis,
@@ -121,7 +121,7 @@ handle_issue (const firehose::analysis &analysis,
   print_any_trace (issue);
 }
 
-/* FIXME.  */
+/* Emit a diagnostic for INFO.  */
 
 static void
 handle_info (const firehose::analysis &analysis,
@@ -135,7 +135,7 @@ handle_info (const firehose::analysis &analysis,
 		 info.m_message);
 }
 
-/* FIXME.  */
+/* Emit a diagnostic for FAILURE.  */
 
 static void
 handle_failure (const firehose::analysis &analysis,
@@ -251,7 +251,9 @@ struct thread_result
   char *m_err;
 };
 
-/* FIXME.  */
+/* A particular checker to run.
+   A "checker" is an executable which takes GCC-style command-line
+   arguments and writes a Firehose JSON file to stdout.  */
 
 class checker
 {
@@ -277,7 +279,7 @@ class checker
   pthread_t m_tid;
 };
 
-/* FIXME.  */
+/* A policy, listing which checkers to run.  */
 
 class policy
 {
@@ -332,9 +334,8 @@ checker::run_checker_thread (void *ptr)
   return ch->run_in_thread ();
 }
 
-/* FIXME.  */
-
-// This is run on the main thread
+/* Create a thread for this checker, calling run_in_thread within it.
+   This is run on the main thread, called by checkers_start.  */
 
 void
 checker::start ()
@@ -345,11 +346,10 @@ checker::start ()
 		  this);
 }
 
-// FIXME: hacking in -lpthread for now
-
-/* FIXME.  */
+/* Wait for this checker's thread to finish, and call
+   handle_json on the UTF-8 JSON result.
 
-// This is run on the main thread
+   This is run on the main thread, called by checkers_finish.  */
 
 void
 checker::finish ()
@@ -357,7 +357,7 @@ checker::finish ()
   /* Wait for the thread to finish.  */
   void *retval;
   if (!pthread_join (m_tid, &retval))
-    ; // FIXME
+    ; // FIXME: error-handling
   thread_result *result = static_cast <thread_result *> (retval);
 
   /* Process the output.  */
@@ -372,20 +372,23 @@ checker::finish ()
   delete result;
 }
 
-/* FIXME.  */
-// This is run within the per-checker thread
+/* This is the "main" routine of the per-checker thread when
+   run in multithreaded mode.
+   Attempt to run the checker in a subprocess, and return the stdout
+   and any error messages.  */
 
 thread_result *
 checker::run_in_thread ()
 {
-  // FIXME: error-handling in capture_stdout isn't fully thread-safe yet
   char *err = NULL;
   char *utf8_buffer = capture_stdout (err);
 
   return new thread_result (utf8_buffer, err);
 }
 
-/* FIXME.  */
+/* This is the single-threaded way to invoke a checker.
+   Run the checker in a subprocess, capture its stdout as UTF-8 JSON,
+   and call handle_json on it.  */
 
 void
 checker::run_single_threaded ()
@@ -430,9 +433,6 @@ checker::capture_stdout (char *&out_err)
 
   obj = pex_init (0, progname, NULL);
 
-  /* FIXME: avoid using known filename; read direct from stdout.  */
-  //char *outname = ;
-
   errmsg = pex_run (obj,
 		    PEX_SEARCH | PEX_USE_PIPES, /* int flags, */
 		    m_executable, /* const char *executable, */
@@ -478,8 +478,9 @@ checker::capture_stdout (char *&out_err)
   return utf8_buffer;
 }
 
-/* FIXME.
-   This can be run within the per-checker thread.  */
+/* Subroutine of checker::capture_stdout.
+   This can be run either on the main thread, or within the
+   per-checker thread.  */
 
 void
 checker::make_args (auto_argvec &out) const
@@ -505,7 +506,10 @@ policy::~policy ()
     delete checker;
 }
 
-/* FIXME.  */
+/* Load policy from the JSON file at PATH.
+   If successful, return true.
+   Othewise, return false, writing to out_err (the caller
+   must free the string).  */
 
 bool
 policy::read_from_file (const char *path, char *&out_err)
@@ -621,7 +625,7 @@ namespace selftest {
 
 /* Selftests.  */
 
-/* FIXME.  */
+/* Verify that we can load a policy file.  */
 
 static void
 test_policy_parsing ()
diff --git a/gcc/firehose.c b/gcc/firehose.c
index 14235b8..b51c662 100644
--- a/gcc/firehose.c
+++ b/gcc/firehose.c
@@ -1,4 +1,4 @@
-/* FIXME.
+/* Serialization format for checker results.
    Copyright (C) 2017 Free Software Foundation, Inc.
 
 This file is part of GCC.
-- 
1.8.5.3

