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>
This commit is contained in:
Peter Krempa 2022-02-14 16:39:00 +01:00
parent 7c35c483ea
commit 654486bd57
14 changed files with 23 additions and 25 deletions

View File

@ -116,6 +116,7 @@ VC_LIST_ALWAYS_EXCLUDE_REGEX = \
# the safewrite wrapper. # the safewrite wrapper.
sc_avoid_write: sc_avoid_write:
@prohibit='\<write *\(' \ @prohibit='\<write *\(' \
exclude='sc_avoid_write' \
in_vc_files='\.c$$' \ in_vc_files='\.c$$' \
halt='consider using safewrite instead of write' \ halt='consider using safewrite instead of write' \
$(_sc_search_regexp) $(_sc_search_regexp)
@ -1569,10 +1570,7 @@ sc_prohibit_enum_impl_with_vir_prefix_in_virsh:
# List all syntax-check exemptions: # List all syntax-check exemptions:
exclude_file_name_regexp--sc_avoid_strcase = ^tools/vsh\.h$$ 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 exclude_file_name_regexp--sc_avoid_write = ^src/libvirt-stream\.c$$
_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_bindtextdomain = .* exclude_file_name_regexp--sc_bindtextdomain = .*

View File

@ -1104,7 +1104,7 @@ int main(int argc, char **argv) {
*/ */
if (statuswrite != -1) { if (statuswrite != -1) {
char status = 0; char status = 0;
while (write(statuswrite, &status, 1) == -1 && while (write(statuswrite, &status, 1) == -1 && /* sc_avoid_write */
errno == EINTR) errno == EINTR)
; ;
VIR_FORCE_CLOSE(statuswrite); VIR_FORCE_CLOSE(statuswrite);
@ -1133,7 +1133,7 @@ int main(int argc, char **argv) {
if (ret != 0) { if (ret != 0) {
/* Tell parent of daemon what failed */ /* Tell parent of daemon what failed */
char status = ret; char status = ret;
while (write(statuswrite, &status, 1) == -1 && while (write(statuswrite, &status, 1) == -1 && /* sc_avoid_write */
errno == EINTR) errno == EINTR)
; ;
} }

View File

@ -910,7 +910,7 @@ int main(int argc, char **argv) {
*/ */
if (statuswrite != -1) { if (statuswrite != -1) {
char status = 0; char status = 0;
while (write(statuswrite, &status, 1) == -1 && while (write(statuswrite, &status, 1) == -1 && /* sc_avoid_write */
errno == EINTR) errno == EINTR)
; ;
VIR_FORCE_CLOSE(statuswrite); VIR_FORCE_CLOSE(statuswrite);
@ -939,7 +939,7 @@ int main(int argc, char **argv) {
if (ret != 0) { if (ret != 0) {
/* Tell parent of daemon what failed */ /* Tell parent of daemon what failed */
char status = ret; char status = ret;
while (write(statuswrite, &status, 1) == -1 && while (write(statuswrite, &status, 1) == -1 && /* sc_avoid_write */
errno == EINTR) errno == EINTR)
; ;
} }

View File

@ -1222,7 +1222,7 @@ static void virLXCControllerConsoleIO(int watch, int fd, int events, void *opaqu
} }
rewrite: rewrite:
done = write(fd, buf, *len); done = write(fd, buf, *len); /* sc_avoid_write */
if (done == -1 && errno == EINTR) if (done == -1 && errno == EINTR)
goto rewrite; goto rewrite;
if (done == -1 && errno != EAGAIN) { if (done == -1 && errno != EAGAIN) {

View File

@ -344,7 +344,7 @@ qemuMonitorIOWrite(qemuMonitor *mon)
buf = mon->msg->txBuffer + mon->msg->txOffset; buf = mon->msg->txBuffer + mon->msg->txOffset;
len = mon->msg->txLength - mon->msg->txOffset; len = mon->msg->txLength - mon->msg->txOffset;
if (mon->msg->txFD == -1) if (mon->msg->txFD == -1)
done = write(mon->fd, buf, len); done = write(mon->fd, buf, len); /* sc_avoid_write */
else else
done = qemuMonitorIOWriteWithFD(mon, buf, len, mon->msg->txFD); done = qemuMonitorIOWriteWithFD(mon, buf, len, mon->msg->txFD);

View File

@ -255,7 +255,7 @@ virRemoteSSHHelperEventOnStdout(int watch G_GNUC_UNUSED,
if (events & VIR_EVENT_HANDLE_WRITABLE && if (events & VIR_EVENT_HANDLE_WRITABLE &&
proxy->sockToTerminal.offset) { proxy->sockToTerminal.offset) {
ssize_t done; ssize_t done;
done = write(fd, done = write(fd, /* sc_avoid_write */
proxy->sockToTerminal.data, proxy->sockToTerminal.data,
proxy->sockToTerminal.offset); proxy->sockToTerminal.offset);
if (done < 0) { if (done < 0) {

View File

@ -1623,7 +1623,7 @@ static ssize_t virNetSocketTLSSessionWrite(const char *buf,
void *opaque) void *opaque)
{ {
virNetSocket *sock = 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) { VIR_NET_TLS_HANDSHAKE_COMPLETE) {
ret = virNetTLSSessionWrite(sock->tlsSession, buf, len); ret = virNetTLSSessionWrite(sock->tlsSession, buf, len);
} else { } else {
ret = write(sock->fd, buf, len); ret = write(sock->fd, buf, len); /* sc_avoid_write */
} }
if (ret < 0) { if (ret < 0) {

View File

@ -1794,7 +1794,7 @@ virCommandSendBuffersHandlePoll(virCommand *cmd,
if (i == virCommandGetNumSendBuffers(cmd)) if (i == virCommandGetNumSendBuffers(cmd))
return 0; return 0;
done = write(fds->fd, done = write(fds->fd, /* sc_avoid_write */
cmd->sendBuffers[i].buffer + cmd->sendBuffers[i].offset, cmd->sendBuffers[i].buffer + cmd->sendBuffers[i].offset,
cmd->sendBuffers[i].buflen - cmd->sendBuffers[i].offset); cmd->sendBuffers[i].buflen - cmd->sendBuffers[i].offset);
if (done < 0) { if (done < 0) {
@ -2306,7 +2306,7 @@ virCommandProcessIO(virCommand *cmd)
fds[i].fd == cmd->inpipe) { fds[i].fd == cmd->inpipe) {
int done; int done;
done = write(cmd->inpipe, cmd->inbuf + inoff, done = write(cmd->inpipe, cmd->inbuf + inoff, /* sc_avoid_write */
inlen - inoff); inlen - inoff);
if (done < 0) { if (done < 0) {
if (errno == EPIPE) { if (errno == EPIPE) {

View File

@ -850,7 +850,7 @@ static int virFDStreamWrite(virStreamPtr st, const char *bytes, size_t nbytes)
ret = nbytes; ret = nbytes;
} else { } else {
retry: retry:
ret = write(fdst->fd, bytes, nbytes); ret = write(fdst->fd, bytes, nbytes); /* sc_avoid_write */
if (ret < 0) { if (ret < 0) {
VIR_WARNINGS_NO_WLOGICALOP_EQUAL_EXPR VIR_WARNINGS_NO_WLOGICALOP_EQUAL_EXPR
if (errno == EAGAIN || errno == EWOULDBLOCK) { if (errno == EAGAIN || errno == EWOULDBLOCK) {

View File

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

View File

@ -81,7 +81,7 @@ static void *threadMain(void *arg)
static void sigHandler(int sig) 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); signal(sig, SIG_DFL);
raise(sig); raise(sig);
} }

View File

@ -83,7 +83,7 @@ static int make_file(const char *path,
if ((fd = real_open(filepath, O_CREAT|O_WRONLY, 0600)) < 0) if ((fd = real_open(filepath, O_CREAT|O_WRONLY, 0600)) < 0)
goto cleanup; goto cleanup;
if (write(fd, value, strlen(value)) != strlen(value)) if (write(fd, value, strlen(value)) != strlen(value)) /* sc_avoid_write */
goto cleanup; goto cleanup;
ret = 0; ret = 0;

View File

@ -54,7 +54,7 @@ static ssize_t testWrite(const char *buf, size_t len, void *opaque)
{ {
int *fd = 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) static ssize_t testRead(char *buf, size_t len, void *opaque)

View File

@ -316,7 +316,7 @@ virConsoleEventOnStdout(int watch G_GNUC_UNUSED,
con->streamToTerminal.offset) { con->streamToTerminal.offset) {
ssize_t done; ssize_t done;
size_t avail; size_t avail;
done = write(fd, done = write(fd, /* sc_avoid_write */
con->streamToTerminal.data, con->streamToTerminal.data,
con->streamToTerminal.offset); con->streamToTerminal.offset);
if (done < 0) { if (done < 0) {