On Tue, 14.10.14 21:50, Boris Egorov (ego...@linux.com) wrote:
> Minor patch. Fixes defects found by cppcheck: do not check if unsigned
> value is const, reduce scope of a few variables.
>
> Signed-off-by: Boris Egorov <ego...@linux.com>
Hmm, semi-enthusiastic about this patch, see below.
> ---
> src/shared/macro.h | 2 +-
> src/shared/ring.c | 7 ++++---
> 2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/src/shared/macro.h b/src/shared/macro.h
> index 9ee332c..372475b 100644
> --- a/src/shared/macro.h
> +++ b/src/shared/macro.h
> @@ -295,7 +295,7 @@ static inline size_t IOVEC_INCREMENT(struct iovec
> *i, unsigned n, size_t k) {
> for (j = 0; j < n; j++) {
> size_t sub;
> - if (_unlikely_(k <= 0))
> + if (_unlikely_(k == 0))
I think cppcheck is really confused here... I really prefer the <=
checks in this case, even if the type would never allow negative
values, and thus == checks would suffice. Why? simply because this
code makes the logic independent of the precise type, it will always
work correctly.
> +++ b/src/shared/ring.c
> @@ -83,8 +83,6 @@ size_t ring_peek(Ring *r, struct iovec *vec) {
> * return the number of bytes copied.
> */
> size_t ring_copy(Ring *r, void *buf, size_t size) {
> - size_t l;
> -
> assert(r);
> assert(buf);
> @@ -92,6 +90,8 @@ size_t ring_copy(Ring *r, void *buf, size_t size) {
> size = r->used;
> if (size > 0) {
> + size_t l;
> +
> l = r->size - r->start;
> if (size <= l) {
> memcpy(buf, &r->buf[r->start], size);
> @@ -110,7 +110,6 @@ size_t ring_copy(Ring *r, void *buf, size_t size) {
> */
> static int ring_resize(Ring *r, size_t nsize) {
> uint8_t *buf;
> - size_t l;
> assert(r);
> assert(nsize > 0);
> @@ -120,6 +119,8 @@ static int ring_resize(Ring *r, size_t nsize) {
> return -ENOMEM;
> if (r->used > 0) {
> + size_t l;
> +
> l = r->size - r->start;
> if (r->used <= l) {
> memcpy(buf, &r->buf[r->start], r->used);
i you rebase and split out the ring.c changes I'll commit them, though
they don't really matter much....
Thanks,
Lennart
--
Lennart Poettering, Red Hat
_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
