From 213ae43edd8d33b82113afcb97ffd9249f6c6bcb Mon Sep 17 00:00:00 2001 From: Oldes Huhuman Date: Thu, 17 Oct 2024 17:10:46 +0200 Subject: [PATCH] CHANGE: use a handle for the directory port state, similar to a file port resolves: https://github.com/Oldes/Rebol-issues/issues/2628 --- src/core/p-dir.c | 142 +++++++++++++++++----------------------- src/os/win32/dev-file.c | 7 +- 2 files changed, 66 insertions(+), 83 deletions(-) diff --git a/src/core/p-dir.c b/src/core/p-dir.c index 770f78f8aa..f9e58cc1f6 100644 --- a/src/core/p-dir.c +++ b/src/core/p-dir.c @@ -159,100 +159,80 @@ /* ** Internal port handler for file directories. ** +** Note: as Read_Dir is currently using static buffer for the path +** there is no need to free dir.file.path +** +** Using port/data to store intermediate file names. +** ***********************************************************************/ { REBSER *port; + REBSER *target; REBVAL *spec; REBVAL *path; - REBVAL *state; - REBREQ dir; - REBCNT args = 0; + REBVAL *data; + REBREQ *dir; REBINT result; - REBCNT len; - //REBYTE *flags; - port = Validate_Port_Value(port_value); + //Debug_Fmt_("DIR ACTION: %r\n", Get_Action_Word(action)); - *D_RET = *D_ARG(1); - CLEARS(&dir); + port = Validate_Port_With_Request(port_value, RDI_FILE, &dir); - // Validate and fetch relevant PORT fields: - spec = BLK_SKIP(port, STD_PORT_SPEC); - if (!IS_OBJECT(spec)) Trap1(RE_INVALID_SPEC, spec); + // Validate PORT fields: + spec = OFV(port, STD_PORT_SPEC); path = Obj_Value(spec, STD_PORT_SPEC_HEAD_REF); if (!path) Trap1(RE_INVALID_SPEC, spec); - if (IS_URL(path)) path = Obj_Value(spec, STD_PORT_SPEC_FILE_PATH); else if (!IS_FILE(path)) Trap1(RE_INVALID_SPEC, path); - - state = BLK_SKIP(port, STD_PORT_STATE); // if block, then port is open. - //flags = Security_Policy(SYM_FILE, path); + *D_RET = *D_ARG(1); - // Get or setup internal state data: - dir.port = port; - dir.device = RDI_FILE; + data = BLK_SKIP(port, STD_PORT_DATA); switch (action) { case A_READ: - //Trap_Security(flags[POL_READ], POL_READ, path); - //args = Find_Refines(ds, ALL_READ_REFS); - if (!IS_BLOCK(state)) { // !!! ignores /SKIP and /PART, for now - Init_Dir_Path(&dir, path, 1, POL_READ); - Set_Block(state, Make_Block(7)); // initial guess - result = Read_Dir(&dir, VAL_SERIES(state)); - ///OS_FREE(dir.file.path); - + // !!! ignores /SKIP and /PART, for now !!! + if (!IS_BLOCK(data)) { + Init_Dir_Path(dir, path, 1, POL_READ); + Set_Block(data, Make_Block(7)); // initial guess + result = Read_Dir(dir, VAL_SERIES(data)); + // don't throw an error if the original path contains wildcard chars * or ? if (result < 0 && !(result == -RFE_OPEN_FAIL && WILD_PATH(path)) ) { - Trap_Port(RE_CANNOT_OPEN, port, dir.error); + Trap_Port(RE_CANNOT_OPEN, port, dir->error); } - - *D_RET = *state; - SET_NONE(state); - } else { - len = VAL_BLK_LEN(state); - // !!? Why does this need to copy the block?? - Set_Block(D_RET, Copy_Block_Values(VAL_SERIES(state), 0, len, TS_STRING)); } + *D_RET = *data; + SET_NONE(data); break; case A_CREATE: - //Trap_Security(flags[POL_WRITE], POL_WRITE, path); - if (IS_BLOCK(state)) Trap1(RE_ALREADY_OPEN, path); // already open create: - Init_Dir_Path(&dir, path, 0, POL_WRITE | REMOVE_TAIL_SLASH); // Sets RFM_DIR too - result = OS_DO_DEVICE(&dir, RDC_CREATE); - ///OS_FREE(dir.file.path); + if (IS_OPEN(dir)) Trap1(RE_ALREADY_OPEN, path); // already open + Init_Dir_Path(dir, path, 0, POL_WRITE | REMOVE_TAIL_SLASH); // Sets RFM_DIR too + SET_NONE(data); + result = OS_DO_DEVICE(dir, RDC_CREATE); if (result < 0) Trap1(RE_NO_CREATE, path); + SET_OPEN(dir); if (action == A_CREATE) return R_ARG2; - SET_NONE(state); break; case A_RENAME: - if (IS_BLOCK(state)) Trap1(RE_ALREADY_OPEN, path); // already open - else { - REBSER *target; - - Init_Dir_Path(&dir, path, 0, POL_WRITE | REMOVE_TAIL_SLASH); // Sets RFM_DIR too - // Convert file name to OS format: - if (!(target = Value_To_OS_Path(D_ARG(2), TRUE))) Trap1(RE_BAD_FILE_PATH, D_ARG(2)); - dir.data = BIN_DATA(target); - OS_DO_DEVICE(&dir, RDC_RENAME); - Free_Series(target); - if (dir.error) Trap1(RE_NO_RENAME, path); - } + Init_Dir_Path(dir, path, 0, POL_WRITE | REMOVE_TAIL_SLASH); // Sets RFM_DIR too + // Convert file name to OS format: + if (!(target = Value_To_OS_Path(D_ARG(2), TRUE))) Trap1(RE_BAD_FILE_PATH, D_ARG(2)); + dir->data = BIN_DATA(target); + OS_DO_DEVICE(dir, RDC_RENAME); + Free_Series(target); + if (dir->error) Trap1(RE_NO_RENAME, path); break; case A_DELETE: - //Trap_Security(flags[POL_WRITE], POL_WRITE, path); - SET_NONE(state); - Init_Dir_Path(&dir, path, 0, POL_WRITE); - // !!! add *.reb deletion - // !!! add recursive delete (?) - result = OS_DO_DEVICE(&dir, RDC_DELETE); - ///OS_FREE(dir.file.path); + SET_NONE(data); + Init_Dir_Path(dir, path, 0, POL_WRITE); + result = OS_DO_DEVICE(dir, RDC_DELETE); + SET_CLOSED(dir); if (result >= 0) return R_ARG2; if (result == -2) return R_FALSE; // else... @@ -260,50 +240,48 @@ break; case A_OPEN: - // !! If open fails, what if user does a READ w/o checking for error? - if (IS_BLOCK(state)) Trap1(RE_ALREADY_OPEN, path); // already open - //Trap_Security(flags[POL_READ], POL_READ, path); - args = Find_Refines(ds, ALL_OPEN_REFS); - if (args & AM_OPEN_NEW) goto create; - //if (args & ~AM_OPEN_READ) Trap1(RE_INVALID_SPEC, path); - Set_Block(state, Make_Block(7)); - Init_Dir_Path(&dir, path, 1, POL_READ); - result = Read_Dir(&dir, VAL_SERIES(state)); - ///OS_FREE(dir.file.path); - if (result < 0) Trap_Port(RE_CANNOT_OPEN, port, dir.error); + if (D_REF(ARG_OPEN_NEW)) goto create; + // If port is already open, just ignore it. + Init_Dir_Path(dir, path, 1, POL_READ); + Set_Block(data, Make_Block(7)); + result = Read_Dir(dir, VAL_SERIES(data)); + if (result < 0) Trap_Port(RE_CANNOT_OPEN, port, dir->error); + SET_OPEN(dir); break; case A_OPENQ: - if (IS_BLOCK(state)) return R_TRUE; + // as read reopens ports, does it make sense to return false here? + if (IS_OPEN(dir)) return R_TRUE; return R_FALSE; case A_CLOSE: - SET_NONE(state); + if (IS_OPEN(dir)) { + if (dir->handle) OS_DO_DEVICE(dir, RDC_CLOSE); + SET_NONE(data); + SET_CLOSED(dir); + } break; case A_QUERY: - //Trap_Security(flags[POL_READ], POL_READ, path); if (IS_NONE(D_ARG(ARG_QUERY_FIELD))) { Ret_File_Modes(port, D_RET); return R_RET; } - SET_NONE(state); - Init_Dir_Path(&dir, path, -1, POL_READ); - if (OS_DO_DEVICE(&dir, RDC_QUERY) < 0) return R_NONE; - Ret_Query_File(port, &dir, D_RET, D_ARG(ARG_QUERY_FIELD)); - ///OS_FREE(dir.file.path); + SET_NONE(data); + Init_Dir_Path(dir, path, -1, POL_READ); + if (OS_DO_DEVICE(dir, RDC_QUERY) < 0) return R_NONE; + Ret_Query_File(port, dir, D_RET, D_ARG(ARG_QUERY_FIELD)); break; //-- Port Series Actions (only called if opened as a port) case A_LENGTHQ: - len = IS_BLOCK(state) ? VAL_BLK_LEN(state) : 0; - SET_INTEGER(D_RET, len); + SET_INTEGER(D_RET, IS_BLOCK(data) ? VAL_BLK_LEN(data) : 0); break; case A_TAILQ: - if(IS_BLOCK(state)) { - return (VAL_BLK_LEN(state) > 0) ? R_FALSE: R_TRUE; + if(IS_BLOCK(data)) { + return (VAL_BLK_LEN(data) > 0) ? R_FALSE : R_TRUE; } Trap_Port(RE_NOT_OPEN, port, 0); diff --git a/src/os/win32/dev-file.c b/src/os/win32/dev-file.c index 93ca525486..5d9764d5b6 100644 --- a/src/os/win32/dev-file.c +++ b/src/os/win32/dev-file.c @@ -308,7 +308,12 @@ static BOOL Seek_File_64(REBREQ *file) ***********************************************************************/ { if (file->handle) { - CloseHandle((HANDLE)(file->handle)); + if (GET_FLAG(file->modes, RFM_DIR)) { + FindClose((HANDLE)(file->handle)); + } + else { + CloseHandle((HANDLE)(file->handle)); + } file->handle = 0; } return DR_DONE;