syntax-check: sc_avoid_write: Don't use blanket file exceptions

Adding an exception for the whole file usually defeats the purpose of a
syntax check and is also likely to get forgotten once the file is
removed.

In case of the suggestion of using 'safewrite' instead of write even the
comment for safewrite states that the function needs to be used only in
certain cases.

Remove the blanket exceptions for files and use an exclude string
instead. The only instance where we keep the full file exception is for
src/libvirt-stream.c as there are multiple uses in example code in
comments where I couldn't find a nicer targetted wapproach.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk
index b96d126bdc49a669880d271f00c2507cf6fe94e2..9407581d0eb39c9d9c96da8d42b78932f3a220dc 100644 (file)
--- a/build-aux/syntax-check.mk
+++ b/build-aux/syntax-check.mk
@@ -116,6 +116,7 @@ VC_LIST_ALWAYS_EXCLUDE_REGEX = \
 # the safewrite wrapper.
 sc_avoid_write:
        @prohibit='\<write *\(' \
+       exclude='sc_avoid_write' \
        in_vc_files='\.c$$' \
        halt='consider using safewrite instead of write' \
          $(_sc_search_regexp)
@@ -1569,10 +1570,7 @@ sc_prohibit_enum_impl_with_vir_prefix_in_virsh:
 # List all syntax-check exemptions:
 exclude_file_name_regexp--sc_avoid_strcase = ^tools/vsh\.h$$
 
-_src1=libvirt-stream|qemu/qemu_monitor|util/vir(command|file|fdstream)|rpc/virnetsocket|lxc/lxc_controller|locking/lock_daemon|logging/log_daemon|remote/remote_ssh_helper
-_test1=shunloadtest|virnettlscontexttest|virnettlssessiontest|vircgroupmock|commandhelper
-exclude_file_name_regexp--sc_avoid_write = \
-  ^(src/($(_src1))|tools/virsh-console|tests/($(_test1)))\.c$$
+exclude_file_name_regexp--sc_avoid_write = ^src/libvirt-stream\.c$$
 
 exclude_file_name_regexp--sc_bindtextdomain = .*
 

diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
index b44649bfbe2a39b88aeec7d61a44cbcf3e7907e0..455bb15d1d19d9f80b2d2dd09ad92dd2a53de5ff 100644 (file)
--- a/src/locking/lock_daemon.c
+++ b/src/locking/lock_daemon.c
@@ -1104,7 +1104,7 @@ int main(int argc, char **argv) {
      */
     if (statuswrite != -1) {
         char status = 0;
-        while (write(statuswrite, &status, 1) == -1 &&
+        while (write(statuswrite, &status, 1) == -1 && /* sc_avoid_write */
                errno == EINTR)
             ;
         VIR_FORCE_CLOSE(statuswrite);
@@ -1133,7 +1133,7 @@ int main(int argc, char **argv) {
         if (ret != 0) {
             /* Tell parent of daemon what failed */
             char status = ret;
-            while (write(statuswrite, &status, 1) == -1 &&
+            while (write(statuswrite, &status, 1) == -1 && /* sc_avoid_write */
                    errno == EINTR)
                 ;
         }

diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c
index 245df9dbbd4350e4691144ba83367cd0ea730a60..4c6118a980fd71f04fcacb84f4dd0f7bb3cae912 100644 (file)
--- a/src/logging/log_daemon.c
+++ b/src/logging/log_daemon.c
@@ -910,7 +910,7 @@ int main(int argc, char **argv) {
      */
     if (statuswrite != -1) {
         char status = 0;
-        while (write(statuswrite, &status, 1) == -1 &&
+        while (write(statuswrite, &status, 1) == -1 && /* sc_avoid_write */
                errno == EINTR)
             ;
         VIR_FORCE_CLOSE(statuswrite);
@@ -939,7 +939,7 @@ int main(int argc, char **argv) {
         if (ret != 0) {
             /* Tell parent of daemon what failed */
             char status = ret;
-            while (write(statuswrite, &status, 1) == -1 &&
+            while (write(statuswrite, &status, 1) == -1 && /* sc_avoid_write */
                    errno == EINTR)
                 ;
         }

diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index 7e798f7b3f30bb1bdf63b5ba88810a5b27e41ce7..677fa5a4fb2ee9658d0b6ec175a6fae9863eb16b 100644 (file)
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -1222,7 +1222,7 @@ static void virLXCControllerConsoleIO(int watch, int fd, int events, void *opaqu
         }
 
      rewrite:
-        done = write(fd, buf, *len);
+        done = write(fd, buf, *len); /* sc_avoid_write */
         if (done == -1 && errno == EINTR)
             goto rewrite;
         if (done == -1 && errno != EAGAIN) {

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 327d6d978a9154f9734a721a8c8cc61c0b033961..f514998ba5c6b612537a2d750cfa36df43321778 100644 (file)
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -344,7 +344,7 @@ qemuMonitorIOWrite(qemuMonitor *mon)
     buf = mon->msg->txBuffer + mon->msg->txOffset;
     len = mon->msg->txLength - mon->msg->txOffset;
     if (mon->msg->txFD == -1)
-        done = write(mon->fd, buf, len);
+        done = write(mon->fd, buf, len); /* sc_avoid_write */
     else
         done = qemuMonitorIOWriteWithFD(mon, buf, len, mon->msg->txFD);
 

diff --git a/src/remote/remote_ssh_helper.c b/src/remote/remote_ssh_helper.c
index 6403fe1761841ca9d7c1626a35262483811c71c8..b4735027be9afab017d28050cd839a0322af1c9a 100644 (file)
--- a/src/remote/remote_ssh_helper.c
+++ b/src/remote/remote_ssh_helper.c
@@ -255,7 +255,7 @@ virRemoteSSHHelperEventOnStdout(int watch G_GNUC_UNUSED,
     if (events & VIR_EVENT_HANDLE_WRITABLE &&
         proxy->sockToTerminal.offset) {
         ssize_t done;
-        done = write(fd,
+        done = write(fd, /* sc_avoid_write */
                      proxy->sockToTerminal.data,
                      proxy->sockToTerminal.offset);
         if (done < 0) {

diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index 51cab4f80c71e331b6442be9dab14d713cfb13e4..1af2778b97b738802b5d5596e325704174bb2b33 100644 (file)
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -1623,7 +1623,7 @@ static ssize_t virNetSocketTLSSessionWrite(const char *buf,
                                            void *opaque)
 {
     virNetSocket *sock = opaque;
-    return write(sock->fd, buf, len);
+    return write(sock->fd, buf, len); /* sc_avoid_write */
 }
 
 
@@ -1818,7 +1818,7 @@ static ssize_t virNetSocketWriteWire(virNetSocket *sock, const char *buf, size_t
         VIR_NET_TLS_HANDSHAKE_COMPLETE) {
         ret = virNetTLSSessionWrite(sock->tlsSession, buf, len);
     } else {
-        ret = write(sock->fd, buf, len);
+        ret = write(sock->fd, buf, len); /* sc_avoid_write */
     }
 
     if (ret < 0) {

diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index 3b8c1c65c160b82aee987873932e18f2a40b65fd..41cf552d7b85a60337dbf5a0a858567711548bf6 100644 (file)
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -1794,7 +1794,7 @@ virCommandSendBuffersHandlePoll(virCommand *cmd,
     if (i == virCommandGetNumSendBuffers(cmd))
         return 0;
 
-    done = write(fds->fd,
+    done = write(fds->fd, /* sc_avoid_write */
                  cmd->sendBuffers[i].buffer + cmd->sendBuffers[i].offset,
                  cmd->sendBuffers[i].buflen - cmd->sendBuffers[i].offset);
     if (done < 0) {
@@ -2306,7 +2306,7 @@ virCommandProcessIO(virCommand *cmd)
                 fds[i].fd == cmd->inpipe) {
                 int done;
 
-                done = write(cmd->inpipe, cmd->inbuf + inoff,
+                done = write(cmd->inpipe, cmd->inbuf + inoff, /* sc_avoid_write */
                              inlen - inoff);
                 if (done < 0) {
                     if (errno == EPIPE) {

diff --git a/src/util/virfdstream.c b/src/util/virfdstream.c
index e2c6461ded7e3d479e614ed65c1f21c76932fc3a..b596659702c0ce7d65177f539b8bedd36b1a19df 100644 (file)
--- a/src/util/virfdstream.c
+++ b/src/util/virfdstream.c
@@ -850,7 +850,7 @@ static int virFDStreamWrite(virStreamPtr st, const char *bytes, size_t nbytes)
         ret = nbytes;
     } else {
      retry:
-        ret = write(fdst->fd, bytes, nbytes);
+        ret = write(fdst->fd, bytes, nbytes); /* sc_avoid_write */
         if (ret < 0) {
             VIR_WARNINGS_NO_WLOGICALOP_EQUAL_EXPR
             if (errno == EAGAIN || errno == EWOULDBLOCK) {

diff --git a/src/util/virfile.c b/src/util/virfile.c
index 5d6f14ba7e4acb113ce00b15c4c57926dad66aea..a04f888e06e788419de3b1eb5e1b40de64fc52ab 100644 (file)
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -1112,17 +1112,17 @@ saferead(int fd, void *buf, size_t count)
     return nread;
 }
 
-/* Like write(), but restarts after EINTR. Doesn't play
- * nicely with nonblocking FD and EAGAIN, in which case
- * you want to use bare write(). Or even use virSocket()
- * if the FD is related to a socket rather than a plain
+/* Like write(), but restarts after EINTR. Encouraged by sc_avoid_write.
+ * Doesn't play nicely with nonblocking FD and EAGAIN, in which case
+ * you want to use bare write() and mark it's use with sc_avoid_write.
+ * Or even use virSocket() if the FD is related to a socket rather than a plain
  * file or pipe. */
 ssize_t
 safewrite(int fd, const void *buf, size_t count)
 {
     size_t nwritten = 0;
     while (count > 0) {
-        ssize_t r = write(fd, buf, count);
+        ssize_t r = write(fd, buf, count); /* sc_avoid_write */
 
         if (r < 0 && errno == EINTR)
             continue;

diff --git a/tests/shunloadtest.c b/tests/shunloadtest.c
index d4ab3cd5acf12d520567f3bd8cf032069b0884f1..724532a3ea641688f6dfa69bbc15549165931b82 100644 (file)
--- a/tests/shunloadtest.c
+++ b/tests/shunloadtest.c
@@ -81,7 +81,7 @@ static void *threadMain(void *arg)
 
 static void sigHandler(int sig)
 {
-    ignore_value(write(STDERR_FILENO, "FAIL\n", 5));
+    ignore_value(write(STDERR_FILENO, "FAIL\n", 5)); /* sc_avoid_write */
     signal(sig, SIG_DFL);
     raise(sig);
 }

diff --git a/tests/vircgroupmock.c b/tests/vircgroupmock.c
index a67c0d95d7417e34682471f0a4ed5478b869e6e4..cebf1912f77877636f39bf4c28cc339fc1a5d95a 100644 (file)
--- a/tests/vircgroupmock.c
+++ b/tests/vircgroupmock.c
@@ -83,7 +83,7 @@ static int make_file(const char *path,
     if ((fd = real_open(filepath, O_CREAT|O_WRONLY, 0600)) < 0)
         goto cleanup;
 
-    if (write(fd, value, strlen(value)) != strlen(value))
+    if (write(fd, value, strlen(value)) != strlen(value)) /* sc_avoid_write */
         goto cleanup;
 
     ret = 0;

diff --git a/tests/virnettlssessiontest.c b/tests/virnettlssessiontest.c
index b9249cca565487e6e5a6ec802e8d4497ab289f6d..9e0d6a46e669310b85779d421f7a5049dd71f823 100644 (file)
--- a/tests/virnettlssessiontest.c
+++ b/tests/virnettlssessiontest.c
@@ -54,7 +54,7 @@ static ssize_t testWrite(const char *buf, size_t len, void *opaque)
 {
     int *fd = opaque;
 
-    return write(*fd, buf, len);
+    return write(*fd, buf, len); /* sc_avoid_write */
 }
 
 static ssize_t testRead(char *buf, size_t len, void *opaque)

diff --git a/tools/virsh-console.c b/tools/virsh-console.c
index 67ee60870695e66e58d714cd1e65e7d12b934cb2..b8780c714d9ae804a51ce16addf117b453e4b02a 100644 (file)
--- a/tools/virsh-console.c
+++ b/tools/virsh-console.c
@@ -316,7 +316,7 @@ virConsoleEventOnStdout(int watch G_GNUC_UNUSED,
         con->streamToTerminal.offset) {
         ssize_t done;
         size_t avail;
-        done = write(fd,
+        done = write(fd, /* sc_avoid_write */
                      con->streamToTerminal.data,
                      con->streamToTerminal.offset);
         if (done < 0) {