Well run projects I have contributed to have linters which fail on bad code style. Ask the submitter to make the linter happy before you review the code.
> Well run projects I have contributed to have linters which fail on bad code style. Ask the submitter to make the linter happy before you review the code.
Linters can't catch most things that are not syntax-style; i.e. linters can't catch semantic style.
Here is code CC generated this morning for me:
size_t bvks_tls_read (bvks_tls_t *tls, void *dst, size_t dst_len)
{
int ret = wolfSSL_read (tls->ssl, dst, (int)dst_len);
if (ret > 0)
return (size_t)ret;
if (ret == 0)
return 0;
// ret < 0, error case
int err = wolfSSL_get_error (tls->ssl, ret);
switch (err) {
case WOLFSSL_ERROR_WANT_READ:
case WOLFSSL_ERROR_WANT_WRITE:
errno = EAGAIN;
break;
case WOLFSSL_ERROR_SYSCALL:
// errno already set by the underlying socket call
break;
default:
errno = EPROTO;
break;
}
return (size_t)-1;
}
size_t bvks_tls_write (bvks_tls_t *tls, const void *src, size_t src_len)
{
int ret = wolfSSL_write (tls->ssl, src, (int)src_len);
if (ret > 0)
return (size_t)ret;
if (ret == 0)
return 0;
// ret < 0, error case
int err = wolfSSL_get_error (tls->ssl, ret);
switch (err) {
case WOLFSSL_ERROR_WANT_READ:
case WOLFSSL_ERROR_WANT_WRITE:
errno = EAGAIN;
break;
case WOLFSSL_ERROR_SYSCALL:
// errno already set by the underlying socket call
break;
default:
errno = EPROTO;
break;
}
return (size_t)-1;
}
One of those is both hard to maintain and has precision (and potential overflow) bugs.
The other isolates the potentially buggy behaviour, validates it, and ensures that future changes to fix size_t/int precision loss bugs only has to be done in a single spot.
No linter is catching that style. It's more "coding style" than "syntax style".