[Asterisk-code-review] stasis recording/stored: remove calls to deprecated readdir ... (asterisk[13])

[Mark Michelson]

Mark Michelson has posted comments on this change. ( https://gerrit.asterisk.org/4224 )

Change subject: stasis_recording/stored: remove calls to deprecated readdir_r function.
......................................................................


Patch Set 2: Code-Review-1

(2 comments)

https://gerrit.asterisk.org/#/c/4224/2/main/file.c
File main/file.c:

PS2, Line 1148: 		/*
              : 		 * It is tempting to use the d_type field found on the dirent
              : 		 * structure. However, that field is not as portable as using
              : 		 * the stat function.
              : 		 */
Would it be possible to only  fall back to using stat() if entry->d_type == DT_UNKNOWN ? Using entry->d_type, while not portable, would probably be far less expensive than using stat() if it's available.


https://gerrit.asterisk.org/#/c/4224/2/tests/test_file.c
File tests/test_file.c:

PS2, Line 63: 	size = strlen(mod_dir);
            : 	if (mod_dir[size - 1] == '/') {
            : 		mod_dir[size - 1] = '\0';
            : 	}
            : 
            : 	if (!(p = strrchr(mod_dir, '/'))) {
            : 		return AST_TEST_FAIL;
            : 	}
            : 	*p++ = '\0';
            : 
            : 	return ast_file_read_dirs(mod_dir, handle_find_file, "test_file.so",
            : 				  2) == FOUND ? AST_TEST_PASS : AST_TEST_FAIL;
This test is making some assumptions about the directory structure for Asterisk modules. Currently, the test will fail if the module directory is something like:

/
/ast_modules/

The first case will fail because of the strrchr() check. The second case will fail because you end up passing an empty string to ast_file_read_dirs()

You're better off sandboxing your own directories and files somewhere on the file system where Asterisk has permission to write. That way, you can guarantee the directory structure and the location of whatever file you're searching for.


-- 
To view, visit https://gerrit.asterisk.org/4224
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id8f54689b1e2873e82a09d0d0d2faf41964e80ba
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-HasComments: Yes