Skip to content

Commit a0ef5ac

Browse files
stevenenglerstevenengler
authored
Merge pull request #1257 from stevenengler/python
Fix descriptor bugs and support dup() on regular files for Python
2 parents 22d4722 + 28c56e7 commit a0ef5ac

10 files changed

Lines changed: 117 additions & 18 deletions

File tree

‎src/main/host/descriptor/descriptor.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ void descriptor_init(LegacyDescriptor* descriptor, LegacyDescriptorType type,
2323
MAGIC_INIT(funcTable);
2424
descriptor->funcTable = funcTable;
2525
descriptor->type = type;
26+
descriptor->handle = -1;
2627
descriptor->listeners = g_hash_table_new_full(
2728
g_direct_hash, g_direct_equal, NULL, (GDestroyNotify)statuslistener_unref);
2829
descriptor->referenceCount = 1;

‎src/main/host/descriptor/file.c‎

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,38 @@ File* file_new() {
149149
return file;
150150
}
151151

152+
File* file_dup(File* file, int* dupError) {
153+
MAGIC_ASSERT(file);
154+
155+
int newFd;
156+
157+
// only dup the os fd if it's valid
158+
if (file->osfile.fd >= 0) {
159+
newFd = dup(file->osfile.fd);
160+
161+
if (newFd < 0) {
162+
*dupError = errno;
163+
return NULL;
164+
}
165+
} else {
166+
newFd = file->osfile.fd;
167+
}
168+
169+
File* newFile = file_new();
170+
171+
newFile->type = file->type;
172+
173+
newFile->osfile.fd = newFd;
174+
newFile->osfile.flags = file->osfile.flags;
175+
newFile->osfile.mode = file->osfile.mode;
176+
newFile->osfile.abspath = strdup(file->osfile.abspath);
177+
178+
// CLOEXEC is a descriptor flag and it is not copied during a dup()
179+
newFile->osfile.flags &= ~O_CLOEXEC;
180+
181+
return newFile;
182+
}
183+
152184
static char* _file_getConcatStr(const char* prefix, const char sep, const char* suffix) {
153185
char* path = NULL;
154186
if (asprintf(&path, "%s%c%s", prefix, sep, suffix) < 0) {
@@ -235,6 +267,11 @@ int file_openat(File* file, File* dir, const char* pathname, int flags, mode_t m
235267
}
236268
#endif
237269

270+
int fd = _file_getFD(file);
271+
if (fd < 0) {
272+
error("Cannot openat() on an unregistered descriptor object with fd %d", fd);
273+
}
274+
238275
/* The default case is a regular file. We do this first so that we have
239276
* an absolute path to compare for special files. */
240277
char* abspath = _file_getPath(file, dir, pathname, workingDir);
@@ -292,7 +329,8 @@ int file_openat(File* file, File* dir, const char* pathname, int flags, mode_t m
292329
/* The os-backed file is now ready. */
293330
descriptor_adjustStatus(&file->super, STATUS_DESCRIPTOR_ACTIVE, TRUE);
294331

295-
return _file_getFD(file);
332+
/* We checked above that fd is non-negative. */
333+
return fd;
296334
}
297335

298336
int file_open(File* file, const char* pathname, int flags, mode_t mode, const char* workingDir) {

‎src/main/host/descriptor/file.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ typedef struct _File File;
3939
// ************************
4040

4141
File* file_new(); // Close the file with descriptor_close()
42+
File* file_dup(File* file, int* dupError);
4243
int file_open(File* file, const char* pathname, int flags, mode_t mode, const char* workingDir);
4344
int file_openat(File* file, File* dir, const char* pathname, int flags, mode_t mode,
4445
const char* workingDir);

‎src/main/host/process.c‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -372,6 +372,9 @@ static File* _process_openStdIOFileHelper(Process* proc, int fd, gchar* fileName
372372

373373
File* stdfile = file_new();
374374

375+
CompatDescriptor* compatDesc = compatdescriptor_fromLegacy((LegacyDescriptor*)stdfile);
376+
descriptortable_set(proc->descTable, fd, compatDesc);
377+
375378
char* cwd = getcwd(NULL, 0);
376379
if (!cwd) {
377380
error("getcwd unable to allocate string buffer, error %i: %s", errno, strerror(errno));
@@ -383,15 +386,10 @@ static File* _process_openStdIOFileHelper(Process* proc, int fd, gchar* fileName
383386

384387
if (errcode < 0) {
385388
error("Opening %s: %s", fileName, strerror(-errcode));
386-
/* Unref and free the file object. */
387-
descriptor_close((LegacyDescriptor*)stdfile);
388-
} else {
389-
debug("Successfully opened fd %d at %s", fd, fileName);
390-
391-
CompatDescriptor* compatDesc = compatdescriptor_fromLegacy((LegacyDescriptor*)stdfile);
392-
descriptortable_set(proc->descTable, fd, compatDesc);
393389
}
394390

391+
debug("Successfully opened fd %d at %s", fd, fileName);
392+
395393
return stdfile;
396394
}
397395

‎src/main/host/syscall/file.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
static int _syscallhandler_validateFileHelper(SysCallHandler* sys, int filefd,
2626
File** file_desc_out) {
2727
/* Check that fd is within bounds. */
28-
if (filefd <= 0) {
28+
if (filefd < 0) {
2929
info("descriptor %i out of bounds", filefd);
3030
return -EBADF;
3131
}

‎src/main/host/syscall/fileat.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ static int _syscallhandler_validateDirHelper(SysCallHandler* sys, int dirfd,
3434
*dir_desc_out = NULL;
3535
}
3636
return 0;
37-
} else if (dirfd <= 0) {
37+
} else if (dirfd < 0) {
3838
info("descriptor %i out of bounds", dirfd);
3939
return -EBADF;
4040
}

‎src/main/host/syscall/timerfd.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
static int _syscallhandler_validateTimerHelper(SysCallHandler* sys, int tfd,
2424
Timer** timer_desc_out) {
2525
/* Check that fd is within bounds. */
26-
if (tfd <= 0) {
26+
if (tfd < 0) {
2727
info("descriptor %i out of bounds", tfd);
2828
return -EBADF;
2929
}

‎src/main/host/syscall/unistd.c‎

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,7 @@ SysCallReturn syscallhandler_close(SysCallHandler* sys,
279279
debug("Trying to close fd %i", fd);
280280

281281
/* Check that fd is within bounds. */
282-
if (fd <= 0) {
282+
if (fd < 0) {
283283
return (SysCallReturn){.state = SYSCALL_DONE, .retval.as_i64 = -EBADF};
284284
}
285285

@@ -297,9 +297,33 @@ SysCallReturn syscallhandler_close(SysCallHandler* sys,
297297
}
298298

299299
SysCallReturn syscallhandler_dup(SysCallHandler* sys,
300-
const SysCallArgs* args) {
301-
warning("Cannot dup legacy descriptors");
302-
return (SysCallReturn){.state = SYSCALL_DONE, .retval.as_i64 = -EOPNOTSUPP};
300+
const SysCallArgs* args) {
301+
gint fd = args->args[0].as_i64;
302+
303+
debug("Trying to dup fd %i", fd);
304+
305+
LegacyDescriptor* desc = process_getRegisteredLegacyDescriptor(sys->process, fd);
306+
if (!desc) {
307+
return (SysCallReturn){.state = SYSCALL_DONE, .retval.as_i64 = -EBADF};
308+
}
309+
310+
LegacyDescriptorType dType = descriptor_getType(desc);
311+
312+
if (dType != DT_FILE) {
313+
warning("Cannot dup legacy non-regular-file descriptors");
314+
return (SysCallReturn){.state = SYSCALL_DONE, .retval.as_i64 = -EOPNOTSUPP};
315+
}
316+
317+
int dupError = 0;
318+
File* newFile = file_dup((File*)desc, &dupError);
319+
320+
if (newFile == NULL) {
321+
utility_assert(dupError < 0);
322+
return (SysCallReturn){.state = SYSCALL_DONE, .retval.as_i64 = dupError};
323+
}
324+
325+
int handle = process_registerLegacyDescriptor(sys->process, (LegacyDescriptor*)newFile);
326+
return (SysCallReturn){.state = SYSCALL_DONE, .retval.as_i64 = handle};
303327
}
304328

305329
SysCallReturn syscallhandler_pipe2(SysCallHandler* sys,

‎src/main/host/syscall/unistd.rs‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,12 @@ pub fn dup_helper(
7979
) -> c::SysCallReturn {
8080
debug!("Duping fd {} ({:?})", fd, desc);
8181

82-
// clone the descriptor and register it
83-
let new_desc = CompatDescriptor::New(desc.clone());
82+
// clone the descriptor (but not the flags)
83+
let mut new_desc = desc.clone();
84+
new_desc.set_flags(DescriptorFlags::empty());
85+
86+
// register the descriptor
87+
let new_desc = CompatDescriptor::New(new_desc);
8488
let new_fd = unsafe {
8589
c::process_registerCompatDescriptor(
8690
sys.process,

‎src/test/file/test_file.c‎

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,6 @@ static void _test_fscanf() {
175175
const char wbuf[] = "testfilefscanf";
176176
char rbuf[sizeof(wbuf)] = {0};
177177
FILE* file;
178-
size_t rv;
179178
_set_contents(&adf, wbuf, sizeof(wbuf));
180179
assert_nonnull_errno(file = fopen(adf.name, "r"));
181180
assert_true_errno(fscanf(file, "%s", rbuf) != EOF);
@@ -259,6 +258,39 @@ static void _test_tmpfile() {
259258
assert_nonneg_errno(fclose(file));
260259
}
261260

261+
static void _test_dup() {
262+
g_auto(AutoDeleteFile) adf = _create_auto_file();
263+
char rbuf[3] = {0};
264+
int fd, fd2;
265+
ssize_t rv;
266+
267+
// write "aa" on original fd
268+
assert_nonneg_errno(fd = open(adf.name, O_RDWR));
269+
assert_nonneg_errno(rv = write(fd, "aa", 3));
270+
g_assert_cmpint(rv, ==, 3);
271+
272+
// dup and write "bb" on new fd
273+
assert_nonneg_errno(fd2 = dup(fd));
274+
assert_nonneg_errno(rv = write(fd2, "bb", 3));
275+
g_assert_cmpint(rv, ==, 3);
276+
277+
// reset the file offset for the original fd
278+
lseek(fd, 0, SEEK_SET);
279+
280+
// read "aa" on new fd
281+
assert_nonneg_errno(rv = read(fd2, rbuf, sizeof(rbuf)));
282+
g_assert_cmpint(rv, ==, sizeof(rbuf));
283+
g_assert_cmpstr(rbuf, ==, "aa");
284+
285+
// read "bb" on original fd
286+
assert_nonneg_errno(rv = read(fd, rbuf, sizeof(rbuf)));
287+
g_assert_cmpint(rv, ==, sizeof(rbuf));
288+
g_assert_cmpstr(rbuf, ==, "bb");
289+
290+
assert_nonneg_errno(close(fd));
291+
assert_nonneg_errno(close(fd2));
292+
}
293+
262294
static void _test_iov() {
263295
g_auto(AutoDeleteFile) adf = _create_auto_file();
264296

@@ -450,6 +482,7 @@ int main(int argc, char* argv[]) {
450482

451483
g_test_add_func("/file/dir", _test_dir);
452484
g_test_add_func("/file/tmpfile", _test_tmpfile);
485+
g_test_add_func("/file/dup", _test_dup);
453486

454487
// TODO: debug and fix iov test
455488
// g_test_add_func("/file/iov", _test_iov);

0 commit comments

Comments
 (0)