Skip to content

Commit

Permalink
rewrite (some) gcc-specific code
Browse files Browse the repository at this point in the history
  • Loading branch information
@ghazel
ghazel committed Feb 3, 2010
1 parent 85c6f7b commit 086fafa
Show file tree
Hide file tree
Showing 10 changed files with 995 additions and 131 deletions.
69 changes: 52 additions & 17 deletions binlog.c
View file
Expand Up @@ -126,7 +126,10 @@ static void
binlog_dref(binlog b)
{
if (!b) return;
if (b->refs < 1) return twarnx("refs is zero for binlog: %s", b->path);
if (b->refs < 1) {
twarnx("refs is zero for binlog: %s", b->path);
return;
}

--b->refs;
if (b->refs < 1) {
Expand Down Expand Up @@ -162,35 +165,49 @@ binlog_read_log_file(binlog b, job binlog_jobs)
int version;

r = read(b->fd, &version, sizeof(version));
if (r == -1) return twarn("read()");
if (r == -1) {
twarn("read()");
return;
}
if (r < sizeof(version)) {
return binlog_warn(b, "EOF while reading version record");
binlog_warn(b, "EOF while reading version record");
return;
}

if (version != binlog_version) {
return warnx("%s: binlog version mismatch %d %d", b->path, version,
warnx("%s: binlog version mismatch %d %d", b->path, version,
binlog_version);
return;
}

while (read(b->fd, &namelen, sizeof(size_t)) == sizeof(size_t)) {
if (namelen >= MAX_TUBE_NAME_LEN) {
return binlog_warn(b, "namelen %d exceeds maximum of %d", namelen, MAX_TUBE_NAME_LEN - 1);
binlog_warn(b, "namelen %d exceeds maximum of %d", namelen, MAX_TUBE_NAME_LEN - 1);
return;
}

if (namelen > 0) {
r = read(b->fd, tubename, namelen);
if (r == -1) return twarn("read()");
if (r == -1) {
twarn("read()");
return;
}
if (r < namelen) {
lseek(b->fd, SEEK_CUR, 0);
return binlog_warn(b, "EOF while reading tube name");
binlog_warn(b, "EOF while reading tube name");
return;
}
}

tubename[namelen] = '\0';
r = read(b->fd, &js, job_record_size);
if (r == -1) return twarn("read()");
if (r == -1) {
twarn("read()");
return;
}
if (r < job_record_size) {
return binlog_warn(b, "EOF while reading job record");
binlog_warn(b, "EOF while reading job record");
return;
}

if (!js.id) break;
Expand Down Expand Up @@ -222,16 +239,21 @@ binlog_read_log_file(binlog b, job binlog_jobs)
job_remove(j);
binlog_dref(j->binlog);
job_free(j);
return binlog_warn(b, "EOF while reading job body");
binlog_warn(b, "EOF while reading job body");
return;
}
r = read(b->fd, j->body, js.body_size);
if (r == -1) return twarn("read()");
if (r == -1) {
twarn("read()");
return;
}
if (r < js.body_size) {
warnx("dropping incomplete job %llu", j->id);
job_remove(j);
binlog_dref(j->binlog);
job_free(j);
return binlog_warn(b, "EOF while reading job body");
binlog_warn(b, "EOF while reading job body");
return;
}
}
break;
Expand Down Expand Up @@ -353,7 +375,10 @@ binlog_open(binlog log, size_t *written)

fd = open(log->path, O_WRONLY | O_CREAT, 0400);

if (fd < 0) return twarn("Cannot open binlog %s", log->path);
if (fd < 0) {
twarn("Cannot open binlog %s", log->path);
return;
}

#ifdef HAVE_POSIX_FALLOCATE
{
Expand All @@ -378,7 +403,8 @@ binlog_open(binlog log, size_t *written)
close(fd);
binlog_dref(log);
errno = r;
return twarn("Cannot allocate space for binlog %s", log->path);
twarn("Cannot allocate space for binlog %s", log->path);
return;
}
}
#endif
Expand Down Expand Up @@ -716,7 +742,10 @@ binlog_init(job binlog_jobs)
/* Recover any jobs in old binlogs */

if (stat(binlog_dir, &sbuf) < 0) {
if (mkdir(binlog_dir, 0700) < 0) return twarn("%s", binlog_dir);
if (mkdir(binlog_dir, 0700) < 0) {
twarn("%s", binlog_dir);
return;
}
} else if (!(sbuf.st_mode & S_IFDIR)) {
twarnx("%s", binlog_dir);
return;
Expand All @@ -727,7 +756,10 @@ binlog_init(job binlog_jobs)
if (binlog_index_min) {
for (idx = binlog_index_min; idx <= binlog_index; idx++) {
r = snprintf(path, PATH_MAX, "%s/binlog.%d", binlog_dir, idx);
if (r > PATH_MAX) return twarnx("path too long: %s", binlog_dir);
if (r > PATH_MAX) {
twarnx("path too long: %s", binlog_dir);
return;
}

fd = open(path, O_RDONLY);

Expand All @@ -748,7 +780,10 @@ binlog_init(job binlog_jobs)

/* Set up for writing out new jobs */
n = ensure_free_space(1);
if (!n) return twarnx("error making first writable binlog");
if (!n) {
twarnx("error making first writable binlog");
return;
}

current_binlog = newest_binlog;
}
Expand Down
5 changes: 3 additions & 2 deletions conn.c
View file
Expand Up @@ -37,7 +37,8 @@ static unsigned int tot_conn_ct = 0;
static conn
conn_alloc()
{
return conn_remove(pool.next) ? : malloc(sizeof(struct conn));
conn x = conn_remove(pool.next);
return x ? x : malloc(sizeof(struct conn));
}

static void
Expand Down Expand Up @@ -226,7 +227,7 @@ soonest_job(conn c)

if (soonest == NULL) {
for (j = c->reserved_jobs.next; j != &c->reserved_jobs; j = j->next) {
if (j->deadline_at <= (soonest ? : j)->deadline_at) soonest = j;
if (j->deadline_at <= (soonest ? soonest : j)->deadline_at) soonest = j;
}
}
c->soonest_job = soonest;
Expand Down
3 changes: 2 additions & 1 deletion ms.c
View file
Expand Up @@ -35,7 +35,8 @@ static void
grow(ms a)
{
void **nitems;
size_t ncap = (a->cap << 1) ? : 1;
size_t cap = (a->cap << 1);
size_t ncap = cap ? cap : 1;

nitems = malloc(ncap * sizeof(void *));
if (!nitems) return;
Expand Down
2 changes: 1 addition & 1 deletion net.c
View file
Expand Up @@ -88,7 +88,7 @@ unbrake(evh h)
if (after_startup) twarnx("releasing the brakes");
after_startup = 1;

accept_handler = h ? : accept_handler;
accept_handler = h ? h : accept_handler;
event_set(&listen_evq, listen_socket, EV_READ | EV_PERSIST,
accept_handler, &listen_evq);

Expand Down
2 changes: 1 addition & 1 deletion pq.c
View file
Expand Up @@ -47,7 +47,7 @@ static void
pq_grow(pq q)
{
job *nheap;
unsigned int ncap = q->cap << 1 ? : 1;
unsigned int ncap = q->cap << 1 ? q->cap << 1 : 1;

nheap = malloc(ncap * sizeof(job));
if (!nheap) return;
Expand Down

3 comments on commit 086fafa

@kr
Copy link

@kr kr commented on 086fafa Mar 1, 2010

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is incredible; thank you for starting this huge task! I am worried that I will have a hard time maintaining cross-platform compatibility in the future, since I have no access to windows machines; we shouldn't let your efforts go to waste. I want to bring this up on the mailing list before I merge it in.

@ghazel
Copy link
Owner Author

@ghazel ghazel commented on 086fafa Mar 1, 2010

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The gist of this is actually quite simple. There are a few things to avoid when using GCC:

  • avoid C99. This is easily enforced by just adding -ansi to your compile flags. Not much lost here, just declare variables at the top of scopes.
  • avoid returning something (even void) from a function which has a void return type. No loss of functionality, and does discourage a few cases which were getting a bit out of control: return twarnx("make_conn() failed"), close(cfd), brake();
  • avoid y ? : x. Just use y ? y : x. Even in the case where you have to make a temporary variable, the compiler will probably make the same code anyway.

@ghazel
Copy link
Owner Author

@ghazel ghazel commented on 086fafa Mar 1, 2010

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like -pedantic will help output warning the ternary operator at least, and some other non-portable code. See this long thread about the issue: http://peter.hates-software.com/2004/08/20/6550cefa.html

Please sign in to comment.