[Freeipa-devel] [PATCH] add path_utils to common

[Stephen Gallagher]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 07/14/2009 02:02 PM, John Dennis wrote:
> This is the first of several patches to add code to repository for the
> audit log watching code. I needed a variety of file system path
> manipulation functions which either weren't in standard libraries or
> didn't work as needed. Since this is general utility code it was broken
> out from the rest of the log watch code to permit reuse. The code has
> been heavily beat upon and sails through valgrind without any errors.
> 
> 
> ------------------------------------------------------------------------
> 
> _______________________________________________
> Freeipa-devel mailing list
> Freeipa-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel

Nack.

path_utils_error_string():
PATH_UTILS_ERROR_NOT_FULLY_NORALAIZED should probably be
PATH_UTILS_ERROR_NOT_FULLY_NORMALIZED. Also, there should be
capitalization of "path" in the string there.

Furthermore, should path_utils_error_string() be localized? I'm not
saying in this patch, but it looks like a good candidate.

get_basename():
It would be wise to put in a check that base_name_size < PATH_MAX or you
could get truncation. In that case, I'd think you might want to have
this function return either NULL or better yet an error code.


get_dirname():
You need to check for ERANGE after getcwd(). It's possible that you're
getting a path back from a foreign filesystem that supports longer paths
than PATH_MAX.

get_directory_and_base_name():
Same concerns as get_dirname() and get_basename(). Also, is this
function useful? (I realize it's one fewer copy to tmp_path than calling
get_basename and get_dirname, but it seems like a bit of code waste)

path_concat():
if (dst >= dst_end) should be if (dst > dst_end). You've already
accounted for the null terminator when defining dst_end.

make_path_absolute():
Same concerns as path_concat().


Finally, you have a ton of tabs and trailing spaces hanging around.



- -- 
Stephen Gallagher
RHCE 804006346421761

Looking to carve out IT costs?
www.redhat.com/carveoutcosts/
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAkpc258ACgkQeiVVYja6o6O8DwCcC4R/zSve2n44TLbHNK/UV3NK
VpwAn3EChXubCKGSpKU8+4qzJLGd6R/J
=UCyx
-----END PGP SIGNATURE-----