From 60fc70a8e0bf25d7388fb4c2e31d912c203f561d Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Sun, 18 May 2014 21:56:03 -0700 Subject: [PATCH] Remove dependencies on getdelim and getline. Also, remove update-game-scores's limits on game scores and simplify its file-locking code. * configure.ac (getdelim, getline): Remove. * lib-src/update-game-score.c (struct score_entry): Unify the username and data members to a single user_data member, since they don't need to be changed independently and getdelim and getline aren't helpful. Make the score member char *, not intmax_t, so that scores are not limited to intmax_t. All uses changed. (lose_syserr): A zero errno stands for invalid data in score file. (normalize_integer): New function. (main): Use it. Check for invalid scores. Omit redundant stat check. (read_score): First arg is now a string, not a FILE *. All uses changed. Do not use getdelim or getline; that's way simpler. (read_scores): Read the whole file, and let read_score handle each line. (score_compare): Compare strings representing integers, not integers. (write_scores) [DOS_NT]: Eliminate unnecessary chmod. (lock_file): Simplify locking code, eliminating goto. Check for unlink failure. --- ChangeLog | 5 + configure.ac | 2 +- lib-src/ChangeLog | 22 +++ lib-src/update-game-score.c | 315 +++++++++++++++++------------------- 4 files changed, 177 insertions(+), 167 deletions(-) diff --git a/ChangeLog b/ChangeLog index 769618f31c..91b1b3ebc6 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,8 @@ +2014-05-19 Paul Eggert + + Remove dependencies on getdelim and getline. + * configure.ac (getdelim, getline): Remove. + 2014-05-18 Glenn Morris * configure.ac: Do not bother testing for png in non-graphical builds. diff --git a/configure.ac b/configure.ac index 4cb29c1fbf..2cfc1ad103 100644 --- a/configure.ac +++ b/configure.ac @@ -3466,7 +3466,7 @@ select getpagesize setlocale \ getrlimit setrlimit shutdown getaddrinfo \ strsignal setitimer \ sendto recvfrom getsockname getpeername getifaddrs freeifaddrs \ -gai_strerror getline getdelim sync \ +gai_strerror sync \ getpwent endpwent getgrent endgrent \ cfmakeraw cfsetspeed copysign __executable_start log2) LIBS=$OLD_LIBS diff --git a/lib-src/ChangeLog b/lib-src/ChangeLog index 3fac70b3f7..3ac401b16a 100644 --- a/lib-src/ChangeLog +++ b/lib-src/ChangeLog @@ -1,3 +1,25 @@ +2014-05-19 Paul Eggert + + Remove dependencies on getline and getdelim. + Also, remove update-game-scores's limits on game scores and + simplify its file-locking code. + * update-game-score.c (struct score_entry): Unify the username and + data members to a single user_data member, since they don't need to be + changed independently and getdelim and getline aren't helpful. + Make the score member char *, not intmax_t, so that scores are not + limited to intmax_t. All uses changed. + (lose_syserr): A zero errno stands for invalid data in score file. + (normalize_integer): New function. + (main): Use it. Check for invalid scores. Omit redundant stat check. + (read_score): First arg is now a string, not a FILE *. All uses + changed. Do not use getdelim or getline; that's way simpler. + (read_scores): Read the whole file, and let read_score handle each + line. + (score_compare): Compare strings representing integers, not integers. + (write_scores) [DOS_NT]: Eliminate unnecessary chmod. + (lock_file): Simplify locking code, eliminating goto. + Check for unlink failure. + 2014-05-18 Paul Eggert Port ctags+etags build to Sun C 5.12. diff --git a/lib-src/update-game-score.c b/lib-src/update-game-score.c index cb6fdf7359..7a64cd04f4 100644 --- a/lib-src/update-game-score.c +++ b/lib-src/update-game-score.c @@ -76,9 +76,8 @@ static int unlock_file (const char *filename, void *state); struct score_entry { - intmax_t score; - char *username; - char *data; + char *score; + char *user_data; }; #define MAX_SCORES min (PTRDIFF_MAX, SIZE_MAX / sizeof (struct score_entry)) @@ -102,7 +101,8 @@ lose (const char *msg) static _Noreturn void lose_syserr (const char *msg) { - fprintf (stderr, "%s: %s\n", msg, strerror (errno)); + fprintf (stderr, "%s: %s\n", msg, + errno ? strerror (errno) : "Invalid data in score file"); exit (EXIT_FAILURE); } @@ -137,6 +137,38 @@ get_prefix (bool running_suid, const char *user_prefix) return user_prefix; } +static char * +normalize_integer (char *num) +{ + bool neg; + char *p; + while (*num != '\n' && isspace (*num)) + num++; + neg = *num == '-'; + num += neg || *num == '-'; + + if (*num == '0') + { + while (*++num == '0') + continue; + neg &= !!*num; + num -= !*num; + } + + for (p = num; '0' <= *p && *p <= '9'; p++) + continue; + + if (*p || p == num) + { + errno = 0; + return 0; + } + + if (neg) + *--num = '-'; + return num; +} + int main (int argc, char **argv) { @@ -144,9 +176,8 @@ main (int argc, char **argv) bool running_suid; void *lockstate; char *scorefile; - char *nl; + char *end, *nl, *user, *data; const char *prefix, *user_prefix = NULL; - struct stat buf; struct score_entry *scores; struct score_entry newscore; bool reverse = false; @@ -169,8 +200,8 @@ main (int argc, char **argv) break; case 'm': { - intmax_t m = strtoimax (optarg, 0, 10); - if (m < 0) + intmax_t m = strtoimax (optarg, &end, 10); + if (optarg == end || *end || m < 0) usage (EXIT_FAILURE); max_scores = min (m, MAX_SCORES); } @@ -194,21 +225,26 @@ main (int argc, char **argv) strcat (scorefile, "/"); strcat (scorefile, argv[optind]); - newscore.score = strtoimax (argv[optind + 1], 0, 10); + newscore.score = normalize_integer (argv[optind + 1]); + if (! newscore.score) + { + fprintf (stderr, "%s: Invalid score\n", argv[optind + 1]); + return EXIT_FAILURE; + } - newscore.data = argv[optind + 2]; - if (strlen (newscore.data) > MAX_DATA_LEN) - newscore.data[MAX_DATA_LEN] = '\0'; - nl = strchr (newscore.data, '\n'); + user = get_user_id (); + if (! user) + lose_syserr ("Couldn't determine user id"); + data = argv[optind + 2]; + if (strlen (data) > MAX_DATA_LEN) + data[MAX_DATA_LEN] = '\0'; + nl = strchr (data, '\n'); if (nl) *nl = '\0'; - - newscore.username = get_user_id (); - if (! newscore.username) - lose_syserr ("Couldn't determine user id"); - - if (stat (scorefile, &buf) < 0) - lose_syserr ("Failed to access scores file"); + newscore.user_data = malloc (strlen (user) + 1 + strlen (data) + 1); + if (! newscore.user_data + || sprintf (newscore.user_data, "%s %s", user, data) < 0) + lose_syserr ("Memory exhausted"); if (lock_file (scorefile, &lockstate) < 0) lose_syserr ("Failed to lock scores file"); @@ -244,137 +280,75 @@ main (int argc, char **argv) exit (EXIT_SUCCESS); } -static int -read_score (FILE *f, struct score_entry *score) +static char * +read_score (char *p, struct score_entry *score) { - int c; - if ((c = getc (f)) != EOF) - ungetc (c, f); - if (feof (f)) - return 1; - for (score->score = 0; (c = getc (f)) != EOF && isdigit (c); ) - { - if (INTMAX_MAX / 10 < score->score) - return -1; - score->score *= 10; - if (INTMAX_MAX - (c - '0') < score->score) - return -1; - score->score += c - '0'; - } - while ((c = getc (f)) != EOF - && isspace (c)) - ; - if (c == EOF) - return -1; - ungetc (c, f); -#ifdef HAVE_GETDELIM - { - size_t count = 0; - score->username = 0; - if (getdelim (&score->username, &count, ' ', f) < 1 - || score->username == NULL) - return -1; - /* Trim the space */ - score->username[strlen (score->username)-1] = '\0'; - } -#else - { - ptrdiff_t unameread = 0; - ptrdiff_t unamelen = 30; - char *username = malloc (unamelen); - if (!username) - return -1; - - while ((c = getc (f)) != EOF && c != ' ') - { - if (unameread >= unamelen - 1) - { - ptrdiff_t unamelen_max = min (PTRDIFF_MAX, SIZE_MAX); - if (unamelen <= unamelen_max / 2) - unamelen *= 2; - else if (unamelen < unamelen_max) - unamelen = unamelen_max; - else - { - errno = ENOMEM; - return -1; - } - username = realloc (username, unamelen); - if (!username) - return -1; - } - username[unameread] = c; - unameread++; - } - if (c == EOF) - return -1; - username[unameread] = '\0'; - score->username = username; - } -#endif -#ifdef HAVE_GETLINE - score->data = NULL; - errno = 0; - { - size_t len; - if (getline (&score->data, &len, f) < 0) - return -1; - score->data[strlen (score->data)-1] = '\0'; - } -#else - { - ptrdiff_t cur = 0; - ptrdiff_t len = 16; - char *buf = malloc (len); - if (!buf) - return -1; - while ((c = getc (f)) != EOF - && c != '\n') - { - if (cur >= len-1) - { - if (min (PTRDIFF_MAX, SIZE_MAX) / 2 < len) - { - errno = ENOMEM; - return -1; - } - if (!(buf = realloc (buf, len *= 2))) - return -1; - } - buf[cur] = c; - cur++; - } - score->data = buf; - score->data[cur] = '\0'; - } -#endif - return 0; + score->score = p; + p = strchr (p, ' '); + if (!p) + return p; + *p++ = 0; + score->user_data = p; + p = strchr (p, '\n'); + if (!p) + return p; + *p++ = 0; + return p; } static int read_scores (const char *filename, struct score_entry **scores, ptrdiff_t *count, ptrdiff_t *alloc) { - int readval = -1; - ptrdiff_t scorecount = 0; - ptrdiff_t cursize = 0; - struct score_entry *ret = 0; - struct score_entry entry; + char *p, *filedata; + ptrdiff_t filesize, nread; + struct stat st; FILE *f = fopen (filename, "r"); - int retval = -1; if (!f) return -1; - while ((readval = read_score (f, &entry)) == 0) - if (push_score (&ret, &scorecount, &cursize, &entry) < 0) + if (fstat (fileno (f), &st) != 0) + return -1; + if (! (0 <= st.st_size && st.st_size < min (PTRDIFF_MAX, SIZE_MAX))) + { + errno = EOVERFLOW; + return -1; + } + filesize = st.st_size; + filedata = malloc (filesize + 1); + if (! filedata) + return -1; + nread = fread (filedata, 1, filesize + 1, f); + if (filesize < nread) + { + errno = 0; return -1; - if (readval > 0 && fclose (f) == 0) + } + if (nread < filesize) + filesize = nread; + if (ferror (f) || fclose (f) != 0) + return -1; + filedata[filesize] = 0; + if (strlen (filedata) != filesize) { - *count = scorecount; - *alloc = cursize; - *scores = ret; - retval = 0; + errno = 0; + return -1; } - return retval; + + *scores = 0; + *count = *alloc = 0; + for (p = filedata; p < filedata + filesize; ) + { + struct score_entry entry; + p = read_score (p, &entry); + if (!p) + { + errno = 0; + return -1; + } + if (push_score (scores, count, alloc, &entry) < 0) + return -1; + } + return 0; } static int @@ -382,7 +356,25 @@ score_compare (const void *a, const void *b) { const struct score_entry *sa = (const struct score_entry *) a; const struct score_entry *sb = (const struct score_entry *) b; - return (sb->score > sa->score) - (sb->score < sa->score); + char *sca = sa->score; + char *scb = sb->score; + size_t lena, lenb; + bool nega = *sca == '-'; + bool negb = *scb == '-'; + int diff = nega - negb; + if (diff) + return diff; + if (nega) + { + char *tmp = sca; + sca = scb + 1; + scb = tmp + 1; + } + lena = strlen (sca); + lenb = strlen (scb); + if (lena != lenb) + return lenb < lena ? -1 : 1; + return strcmp (scb, sca); } static int @@ -451,18 +443,12 @@ write_scores (const char *filename, const struct score_entry *scores, if (! f) return -1; for (i = 0; i < count; i++) - if (fprintf (f, "%"PRIdMAX" %s %s\n", - scores[i].score, scores[i].username, scores[i].data) - < 0) + if (fprintf (f, "%s %s\n", scores[i].score, scores[i].user_data) < 0) return -1; if (fclose (f) != 0) return -1; if (rename (tempfile, filename) != 0) return -1; -#ifdef DOS_NT - if (chmod (filename, 0644) < 0) - return -1; -#endif return 0; } @@ -479,30 +465,27 @@ lock_file (const char *filename, void **state) strcpy (lockpath, filename); strcat (lockpath, lockext); *state = lockpath; - trylock: - attempts++; - /* If the lock is over an hour old, delete it. */ - if (stat (lockpath, &buf) == 0 - && 60 * 60 < time (0) - buf.st_ctime) - unlink (lockpath); - fd = open (lockpath, O_CREAT | O_EXCL, 0600); - if (fd < 0) + + while ((fd = open (lockpath, O_CREAT | O_EXCL, 0600)) < 0) { - if (errno == EEXIST) + if (errno != EEXIST) + return -1; + attempts++; + + /* Break the lock if it is over an hour old, or if we've tried + more than MAX_ATTEMPTS times. We won't corrupt the file, but + we might lose some scores. */ + if (MAX_ATTEMPTS < attempts + || (stat (lockpath, &buf) == 0 && 60 * 60 < time (0) - buf.st_ctime)) { - /* Break the lock; we won't corrupt the file, but we might - lose some scores. */ - if (attempts > MAX_ATTEMPTS) - { - unlink (lockpath); - attempts = 0; - } - sleep ((rand () % 2)+1); - goto trylock; + if (unlink (lockpath) != 0 && errno != ENOENT) + return -1; + attempts = 0; } - else - return -1; + + sleep ((rand () & 1) + 1); } + close (fd); return 0; } -- 2.20.1