Skip to content

Commit e1de726

Browse files
committed
Migrate ODB files to new error handling
This migrates odb.c, odb_loose.c, odb_pack.c and pack.c to the new style of error handling. Also got the unix and win32 versions of map.c. There are some minor changes to other files but no others were completely converted. This also contains an update to filebuf so that a zeroed out filebuf will not think that the fd (== 0) is actually open (and inadvertently call close() on fd 0 if cleaned up). Lastly, this was built and tested on win32 and contains a bunch of fixes for the win32 build which was pretty broken.
1 parent dda708e commit e1de726

File tree

21 files changed

+496
-577
lines changed

21 files changed

+496
-577
lines changed

include/git2/errors.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ typedef enum {
103103
GIT_EOBJCORRUPTED = -28,
104104

105105
/** The given short oid is ambiguous */
106-
GIT_EAMBIGUOUSOIDPREFIX = -29,
106+
GIT_EAMBIGUOUS = -29,
107107

108108
/** Skip and passthrough the given ODB backend */
109109
GIT_EPASSTHROUGH = -30,
@@ -128,6 +128,7 @@ typedef enum {
128128
GITERR_REPOSITORY,
129129
GITERR_CONFIG,
130130
GITERR_REGEX,
131+
GITERR_ODB
131132
} git_error_class;
132133

133134
/**

src/blob.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ static int write_file_stream(git_oid *oid, git_odb *odb, const char *path, git_o
7878
char buffer[4096];
7979
git_odb_stream *stream = NULL;
8080

81-
if ((error = git_odb_open_wstream(&stream, odb, file_size, GIT_OBJ_BLOB)) < GIT_SUCCESS)
81+
if ((error = git_odb_open_wstream(&stream, odb, (size_t)file_size, GIT_OBJ_BLOB)) < GIT_SUCCESS)
8282
return error;
8383

8484
if ((fd = p_open(path, O_RDONLY)) < 0) {

src/errors.c

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ static struct {
4040
{GIT_EEXISTS, "A reference with this name already exists"},
4141
{GIT_EOVERFLOW, "The given integer literal is too large to be parsed"},
4242
{GIT_ENOTNUM, "The given literal is not a valid number"},
43-
{GIT_EAMBIGUOUSOIDPREFIX, "The given oid prefix is ambiguous"},
43+
{GIT_EAMBIGUOUS, "The given oid prefix is ambiguous"},
4444
};
4545

4646
const char *git_strerror(int num)
@@ -129,9 +129,29 @@ void giterr_set(int error_class, const char *string, ...)
129129

130130
/* automatically suffix strerror(errno) for GITERR_OS errors */
131131
if (error_class == GITERR_OS) {
132-
strncat(error_str, ": ", sizeof(error_str));
133-
strncat(error_str, strerror(errno), sizeof(error_str));
134-
errno = 0;
132+
if (errno != 0) {
133+
strncat(error_str, ": ", sizeof(error_str));
134+
strncat(error_str, strerror(errno), sizeof(error_str));
135+
errno = 0;
136+
}
137+
#ifdef GIT_WIN32
138+
else {
139+
LPVOID lpMsgBuf;
140+
DWORD dw = GetLastError();
141+
142+
FormatMessage(
143+
FORMAT_MESSAGE_ALLOCATE_BUFFER |
144+
FORMAT_MESSAGE_FROM_SYSTEM |
145+
FORMAT_MESSAGE_IGNORE_INSERTS,
146+
NULL, dw, 0, (LPTSTR) &lpMsgBuf, 0, NULL);
147+
148+
if (lpMsgBuf) {
149+
strncat(error_str, ": ", sizeof(error_str));
150+
strncat(error_str, (const char *)lpMsgBuf, sizeof(error_str));
151+
LocalFree(lpMsgBuf);
152+
}
153+
}
154+
#endif
135155
}
136156

137157
giterr_set_str(error_class, error_str);

src/filebuf.c

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,8 @@ static int lock_file(git_filebuf *file, int flags)
6767
if (file->fd < 0)
6868
return -1;
6969

70+
file->fd_is_open = true;
71+
7072
if ((flags & GIT_FILEBUF_APPEND) && git_path_exists(file->path_original) == true) {
7173
git_file source;
7274
char buffer[2048];
@@ -94,10 +96,10 @@ static int lock_file(git_filebuf *file, int flags)
9496

9597
void git_filebuf_cleanup(git_filebuf *file)
9698
{
97-
if (file->fd >= 0)
99+
if (file->fd_is_open && file->fd >= 0)
98100
p_close(file->fd);
99101

100-
if (file->fd >= 0 && file->path_lock && git_path_exists(file->path_lock) == true)
102+
if (file->fd_is_open && file->path_lock && git_path_exists(file->path_lock))
101103
p_unlink(file->path_lock);
102104

103105
if (file->digest)
@@ -239,6 +241,7 @@ int git_filebuf_open(git_filebuf *file, const char *path, int flags)
239241
git_buf_free(&tmp_path);
240242
goto cleanup;
241243
}
244+
file->fd_is_open = true;
242245

243246
/* No original path */
244247
file->path_original = NULL;
@@ -308,6 +311,7 @@ int git_filebuf_commit(git_filebuf *file, mode_t mode)
308311

309312
p_close(file->fd);
310313
file->fd = -1;
314+
file->fd_is_open = false;
311315

312316
if (p_chmod(file->path_lock, mode)) {
313317
giterr_set(GITERR_OS, "Failed to set attributes for file at '%s'", file->path_lock);

src/filebuf.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ struct git_filebuf {
4040

4141
size_t buf_size, buf_pos;
4242
git_file fd;
43+
bool fd_is_open;
4344
int last_error;
4445
};
4546

src/fileops.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ int git_futils_readbuffer_updated(git_buf *buf, const char *path, time_t *mtime,
117117
return fd;
118118

119119
if (p_fstat(fd, &st) < 0 || S_ISDIR(st.st_mode) || !git__is_sizet(st.st_size+1)) {
120-
close(fd);
120+
p_close(fd);
121121
giterr_set(GITERR_OS, "Invalid regular file stat for '%s'", path);
122122
return -1;
123123
}
@@ -127,7 +127,7 @@ int git_futils_readbuffer_updated(git_buf *buf, const char *path, time_t *mtime,
127127
* has been modified.
128128
*/
129129
if (mtime != NULL && *mtime >= st.st_mtime) {
130-
close(fd);
130+
p_close(fd);
131131
return 0;
132132
}
133133

@@ -139,8 +139,8 @@ int git_futils_readbuffer_updated(git_buf *buf, const char *path, time_t *mtime,
139139
git_buf_clear(buf);
140140

141141
if (git_buf_grow(buf, len + 1) < 0) {
142-
close(fd);
143-
return GIT_ENOMEM;
142+
p_close(fd);
143+
return -1;
144144
}
145145

146146
buf->ptr[len] = '\0';
@@ -149,7 +149,7 @@ int git_futils_readbuffer_updated(git_buf *buf, const char *path, time_t *mtime,
149149
ssize_t read_size = p_read(fd, buf->ptr, len);
150150

151151
if (read_size < 0) {
152-
close(fd);
152+
p_close(fd);
153153
giterr_set(GITERR_OS, "Failed to read descriptor for '%s'", path);
154154
return -1;
155155
}

src/indexer.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,7 @@ int git_indexer_write(git_indexer *idx)
307307
int git_indexer_run(git_indexer *idx, git_indexer_stats *stats)
308308
{
309309
git_mwindow_file *mwf;
310-
off_t off = sizeof(struct git_pack_header);
310+
git_off_t off = sizeof(struct git_pack_header);
311311
int error;
312312
struct entry *entry;
313313
unsigned int left, processed;
@@ -328,18 +328,18 @@ int git_indexer_run(git_indexer *idx, git_indexer_stats *stats)
328328
struct git_pack_entry *pentry;
329329
git_mwindow *w = NULL;
330330
int i;
331-
off_t entry_start = off;
331+
git_off_t entry_start = off;
332332
void *packed;
333333
size_t entry_size;
334334

335-
entry = git__malloc(sizeof(struct entry));
336-
memset(entry, 0x0, sizeof(struct entry));
335+
entry = git__calloc(1, sizeof(*entry));
336+
GITERR_CHECK_ALLOC(entry);
337337

338338
if (off > UINT31_MAX) {
339339
entry->offset = UINT32_MAX;
340340
entry->offset_long = off;
341341
} else {
342-
entry->offset = off;
342+
entry->offset = (uint32_t)off;
343343
}
344344

345345
error = git_packfile_unpack(&obj, idx->pack, &off);
@@ -369,7 +369,7 @@ int git_indexer_run(git_indexer *idx, git_indexer_stats *stats)
369369
git_oid_cpy(&entry->oid, &oid);
370370
entry->crc = crc32(0L, Z_NULL, 0);
371371

372-
entry_size = off - entry_start;
372+
entry_size = (size_t)(off - entry_start);
373373
packed = git_mwindow_open(mwf, &w, entry_start, entry_size, &left);
374374
if (packed == NULL) {
375375
error = git__rethrow(error, "Failed to open window to read packed data");

src/map.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ typedef struct { /* memory mapped buffer */
3131
#endif
3232
} git_map;
3333

34+
extern int validate_map_args(git_map *out, size_t len, int prot, int flags, int fd, git_off_t offset);
35+
3436
extern int p_mmap(git_map *out, size_t len, int prot, int flags, int fd, git_off_t offset);
3537
extern int p_munmap(git_map *map);
3638

src/mwindow.c

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -178,8 +178,10 @@ static git_mwindow *new_window(
178178
* window.
179179
*/
180180

181-
if (git_futils_mmap_ro(&w->window_map, fd, w->offset, (size_t)len) < GIT_SUCCESS)
182-
goto cleanup;
181+
if (git_futils_mmap_ro(&w->window_map, fd, w->offset, (size_t)len) < 0) {
182+
git__free(w);
183+
return NULL;
184+
}
183185

184186
ctl->mmap_calls++;
185187
ctl->open_windows++;
@@ -191,10 +193,6 @@ static git_mwindow *new_window(
191193
ctl->peak_open_windows = ctl->open_windows;
192194

193195
return w;
194-
195-
cleanup:
196-
git__free(w);
197-
return NULL;
198196
}
199197

200198
/*
@@ -253,11 +251,10 @@ unsigned char *git_mwindow_open(
253251
int git_mwindow_file_register(git_mwindow_file *mwf)
254252
{
255253
git_mwindow_ctl *ctl = &GIT_GLOBAL->mem_ctl;
256-
int error;
257254

258255
if (ctl->windowfiles.length == 0 &&
259-
(error = git_vector_init(&ctl->windowfiles, 8, NULL)) < GIT_SUCCESS)
260-
return error;
256+
git_vector_init(&ctl->windowfiles, 8, NULL) < 0)
257+
return -1;
261258

262259
return git_vector_insert(&ctl->windowfiles, mwf);
263260
}

src/object.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ int git_object_lookup_prefix(
9292
assert(repo && object_out && id);
9393

9494
if (len < GIT_OID_MINPREFIXLEN)
95-
return git__throw(GIT_EAMBIGUOUSOIDPREFIX,
95+
return git__throw(GIT_EAMBIGUOUS,
9696
"Failed to lookup object. Prefix length is lower than %d.", GIT_OID_MINPREFIXLEN);
9797

9898
error = git_repository_odb__weakptr(&odb, repo);

0 commit comments

Comments
 (0)