[Ocfs2-tools-devel] [PATCH 06/11] Ocfs2-tools: Add implementation of generic utils function to o2info.

[Joel Becker]

On Fri, Jan 08, 2010 at 04:50:56PM +0800, Tristan Ye wrote:
> +errcode_t o2info_close(struct o2info_method *om)
> +{
> +	errcode_t err = 0;
> +	int rc = 0;
> +
> +	if (om->om_method & O2INFO_USE_LIBOCFS2) {
> +		if (om->om_fs) {
> +			err = ocfs2_close(om->om_fs);
> +			if (err) {
> +				tcom_err(err, "while closing device");
> +				goto out;
> +			}
> +		}
> +	} else {
> +		if (om->om_fd) {

	om_fd can be zero.  Check for om_fd<0.

> +int o2info_is_device(const char *path)
> +{
> +	int rc;
> +	struct stat st;
> +
> +	rc = lstat(path, &st);

	You want stat() or even better fstat().  If you're looking at a
symlink you actually want to follow it, I would think.  Most people
would expect that.

> +	if ((S_ISBLK(st.st_mode)) || (S_ISCHR(st.st_mode)))
> +		rc = 1;

	Remember, a symlink can't be BLK or CHR.

> +int o2info_uid2name(uid_t uid, char **uname)
> +{
> +	struct passwd *entry;
> +	int ret = 0;
> +
> +	entry = getpwuid(uid);
> +
> +	if (!entry) {
> +		errorf("user %d does not exist!\n", uid);
> +		ret = -1;
> +	} else {
> +		*uname = strdup(entry->pw_name);
> +		if (*uname == NULL) {
> +			errorf("No memory for allocation\n");
> +			ret = -1;
> +		}
> +	}
> +
> +	return ret;
> +}
> +
> +int o2info_gid2name(gid_t gid, char **gname)
> +{
> +	struct group *group;
> +	int ret = 0;
> +
> +	group = getgrgid(gid);
> +
> +	if (!group) {
> +		errorf("group %d does not exist!\n", gid);
> +		ret = -1;
> +	} else {
> +		*gname = strdup(group->gr_name);
> +		if (*gname == NULL) {
> +			errorf("No memory for allocation\n");
> +			ret = -1;
> +		}
> +	}
> +
> +	return ret;
> +}

	Here you're using strdup() instead of GString.  If you are going
to use GLib, really use it.  Otherwise, drop it and use standard C
library stuff.  Really, I'd be tempted to have the caller pass in a
buffer, so as to avoid allocation.
	I have to see how the utils are used in later functions, though,
before I can provide more comments on the context.

Joel

-- 

Life's Little Instruction Book #198

	"Feed a stranger's expired parking meter."

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127