Add -Wsign-compare to the build as comparisons between a negative signed
int and a positive unsigned int don't compare as expected. Fix up the
comparisons then flagged. This includes moving to zero-based indices for
the cull tables.
Signed-off-by: David Howells <dhowells@xxxxxxxxxx>
---
Makefile | 2 -
cachefilesd.c | 121 ++++++++++++++++++++++++++++-----------------------------
2 files changed, 60 insertions(+), 63 deletions(-)
diff --git a/Makefile b/Makefile
index 1f3688b..fff0e87 100644
--- a/Makefile
+++ b/Makefile
@@ -1,4 +1,4 @@
-CFLAGS := -g -O2 -Wall
+CFLAGS := -g -O2 -Wall -Wsign-compare
INSTALL := install
DESTDIR :=
ETCDIR := /etc
diff --git a/cachefilesd.c b/cachefilesd.c
index 7d86821..6658ba5 100644
--- a/cachefilesd.c
+++ b/cachefilesd.c
@@ -96,8 +96,8 @@ static unsigned culltable_size = 4096;
static struct object **cullbuild;
static struct object **cullready;
-static int oldest_build = -1;
-static int oldest_ready = -1;
+static unsigned nr_in_build_table;
+static unsigned nr_in_ready_table;
static int ncullable;
@@ -567,10 +567,10 @@ static void open_cache(void)
if (fstatfs(graveyardfd, &sfs) < 0)
oserror("Unable to stat cache filesystem");
- if (sfs.f_bsize == -1 ||
- sfs.f_blocks == -1 ||
- sfs.f_bfree == -1 ||
- sfs.f_bavail == -1)
+ if (sfs.f_bsize + 1 == 0 ||
+ sfs.f_blocks + 1 == 0 ||
+ sfs.f_bfree + 1 == 0 ||
+ sfs.f_bavail + 1 == 0)
error("Backing filesystem returns unusable statistics through fstatfs()");
}
@@ -642,16 +642,16 @@ static void cachefilesd(void)
}
if (cull) {
- if (oldest_ready >= 0)
+ if (nr_in_ready_table > 0)
cull_objects();
- else if (oldest_build < 0)
+ else if (nr_in_build_table == 0)
jumpstart_scan = true;
}
if (scan)
build_cull_table();
- if (!scan && oldest_ready < 0 && oldest_build >= 0)
+ if (!scan && nr_in_ready_table == 0 && nr_in_build_table > 0)
decant_cull_table();
}
@@ -997,21 +997,21 @@ static void insert_into_cull_table(struct object *object)
error("NULL object pointer");
/* just insert if table is empty */
- if (oldest_build == -1) {
+ if (nr_in_build_table == 0) {
object->usage++;
- oldest_build = 0;
cullbuild[0] = object;
+ nr_in_build_table++;
return;
}
/* insert somewhere if table is not full */
- if (oldest_build < culltable_size - 1) {
+ if (nr_in_build_table < culltable_size) {
object->usage++;
- oldest_build++;
/* just insert at end if new oldest object */
- if (object->atime <= cullbuild[oldest_build - 1]->atime) {
- cullbuild[oldest_build] = object;
+ if (object->atime <= cullbuild[nr_in_build_table - 1]->atime) {
+ cullbuild[nr_in_build_table] = object;
+ nr_in_build_table++;
return;
}
@@ -1019,25 +1019,27 @@ static void insert_into_cull_table(struct object *object)
if (object->atime > cullbuild[0]->atime) {
memmove(&cullbuild[1],
&cullbuild[0],
- oldest_build * sizeof(cullbuild[0]));
+ nr_in_build_table * sizeof(cullbuild[0]));
cullbuild[0] = object;
+ nr_in_build_table++;
return;
}
/* if only two objects in list then insert between them */
- if (oldest_build == 2) {
+ if (nr_in_build_table == 2) {
cullbuild[2] = cullbuild[1];
cullbuild[1] = object;
+ nr_in_build_table++;
return;
}
/* insert somewhere in between front and back elements
- * of a three object list
+ * of a three-plus object list
* - oldest_build == #objects_currently_in_list
*/
y = 1;
- o = oldest_build - 1;
+ o = nr_in_build_table - 1;
do {
m = (y + o) / 2;
@@ -1051,14 +1053,15 @@ static void insert_into_cull_table(struct object *object)
memmove(&cullbuild[y + 1],
&cullbuild[y],
- (oldest_build - y) * sizeof(cullbuild[0]));
+ (nr_in_build_table - y) * sizeof(cullbuild[0]));
cullbuild[y] = object;
+ nr_in_build_table++;
return;
}
/* if table is full then insert only if older than newest */
- if (oldest_build > culltable_size - 1)
+ if (nr_in_build_table > culltable_size)
error("Cull table overfull");
if (object->atime >= cullbuild[0]->atime)
@@ -1125,7 +1128,8 @@ static void build_cull_table(void)
struct dirent dirent, *de;
struct object *curr, *child;
struct stat64 st;
- int loop, fd;
+ unsigned loop;
+ int fd;
curr = scan;
@@ -1226,45 +1230,41 @@ next:
goto next;
}
- for (loop = 0; loop <= oldest_ready; loop++)
+ for (loop = 0; loop < nr_in_ready_table; loop++)
if (cullready[loop] == child)
break;
- if (loop == oldest_ready) {
+ if (loop == nr_in_ready_table - 1) {
/* child was oldest object */
- cullready[oldest_ready] = (void *)(0x6b000000 | __LINE__);
- oldest_ready--;
+ cullready[--nr_in_ready_table] = (void *)(0x6b000000 | __LINE__);
put_object(child);
goto removed;
}
- else if (loop < oldest_ready) {
+ else if (loop < nr_in_ready_table - 1) {
/* child was somewhere in between */
memmove(&cullready[loop],
&cullready[loop + 1],
- (oldest_ready - loop) * sizeof(cullready[0]));
- cullready[oldest_ready] = (void *)(0x6b000000 | __LINE__);
- oldest_ready--;
+ (nr_in_ready_table - (loop + 1)) * sizeof(cullready[0]));
+ cullready[--nr_in_ready_table] = (void *)(0x6b000000 | __LINE__);
put_object(child);
goto removed;
}
- for (loop = 0; loop <= oldest_build; loop++)
+ for (loop = 0; loop < nr_in_build_table; loop++)
if (cullbuild[loop] == child)
break;
- if (loop == oldest_build) {
+ if (loop == nr_in_build_table - 1) {
/* child was oldest object */
- cullbuild[oldest_build] = (void *)(0x6b000000 | __LINE__);
- oldest_build--;
+ cullbuild[--nr_in_build_table] = (void *)(0x6b000000 | __LINE__);
put_object(child);
}
- else if (loop < oldest_build) {
+ else if (loop < nr_in_build_table - 1) {
/* child was somewhere in between */
memmove(&cullbuild[loop],
&cullbuild[loop + 1],
- (oldest_build - loop) * sizeof(cullbuild[0]));
- cullbuild[oldest_build] = (void *)(0x6b000000 | __LINE__);
- oldest_build--;
+ (nr_in_build_table - (loop + 1)) * sizeof(cullbuild[0]));
+ cullbuild[--nr_in_build_table] = (void *)(0x6b000000 | __LINE__);
put_object(child);
}
@@ -1357,20 +1357,20 @@ found_unexpected_object:
*/
static void decant_cull_table(void)
{
- int loop, space, avail, copy, leave, n;
+ unsigned loop, avail, copy, leave, space, n;
if (scan)
error("Can't decant cull table whilst scanning");
/* if nothing there, scan again in a short while */
- if (oldest_build < 0) {
+ if (nr_in_build_table == 0) {
signal(SIGALRM, sigalrm);
alarm(30);
return;
}
/* mark the new entries cullable */
- for (loop = 0; loop <= oldest_build; loop++) {
+ for (loop = 0; loop < nr_in_build_table; loop++) {
if (!cullbuild[loop]->cullable) {
cullbuild[loop]->cullable = true;
ncullable++;
@@ -1378,51 +1378,49 @@ static void decant_cull_table(void)
}
/* if the ready table is empty, copy the whole lot across */
- if (oldest_ready == -1) {
- copy = oldest_build + 1;
+ if (nr_in_ready_table == 0) {
+ copy = nr_in_build_table;
debug(1, "Decant (all %d)", copy);
n = copy * sizeof(cullready[0]);
memcpy(cullready, cullbuild, n);
memset(cullbuild, 0x6e, n);
- oldest_ready = oldest_build;
- oldest_build = -1;
+ nr_in_ready_table = nr_in_build_table;
+ nr_in_build_table = 0;
goto check;
}
/* decant some of the build table if there's space */
- space = culltable_size - (oldest_ready + 1);
- if (space <= 0) {
- if (space < 0)
- error("Less than zero space in ready table");
+ if (culltable_size < nr_in_ready_table)
+ error("Less than zero space in ready table");
+ space = culltable_size - nr_in_ready_table;
+ if (space == 0)
goto check;
- }
/* work out how much of the build table we can copy */
- copy = avail = oldest_build + 1;
+ copy = avail = nr_in_build_table;
if (copy > space)
copy = space;
leave = avail - copy;
- debug(1, "Decant (%d/%d to %d)", copy, avail, space);
+ debug(1, "Decant (%u/%u to %u)", copy, avail, space);
/* make a hole in the ready table transfer "copy" elements from the end
* of cullbuild (oldest) to the beginning of cullready (youngest)
*/
- n = oldest_ready + 1;
- memmove(&cullready[copy], &cullready[0], n * sizeof(cullready[0]));
- oldest_ready += copy;
+ memmove(&cullready[copy], &cullready[0], nr_in_ready_table * sizeof(cullready[0]));
+ nr_in_ready_table += copy;
memcpy(&cullready[0], &cullbuild[leave], copy * sizeof(cullready[0]));
memset(&cullbuild[leave], 0x6b, copy * sizeof(cullbuild[0]));
- oldest_build = leave - 1;
+ nr_in_build_table = leave;
if (copy + leave > culltable_size)
error("Scan table exceeded (%d+%d)", copy, leave);
check:
- for (loop = 0; loop < oldest_ready; loop++)
+ for (loop = 0; loop < nr_in_ready_table; loop++)
if (((long)cullready[loop] & 0xf0000000) == 0x60000000)
abort();
}
@@ -1499,14 +1497,13 @@ static void cull_objects(void)
if (ncullable <= 0)
error("Cullable object count is inconsistent");
- if (cullready[oldest_ready]->cullable) {
- cull_object(cullready[oldest_ready]);
- cullready[oldest_ready] = (void *)(0x6b000000 | __LINE__);
- oldest_ready--;
+ if (cullready[nr_in_ready_table - 1]->cullable) {
+ cull_object(cullready[nr_in_ready_table - 1]);
+ cullready[--nr_in_ready_table] = (void *)(0x6b000000 | __LINE__);
}
/* must start refilling the cull table */
- if (!scan && oldest_build <= culltable_size / 2 + 2) {
+ if (!scan && nr_in_build_table < culltable_size / 2 + 2) {
decant_cull_table();
debug(1, "Refilling cull table");
--
Linux-cachefs mailing list
Linux-cachefs@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/linux-cachefs
