[EMAIL PROTECTED] wrote on Wed, 06 Dec 2006 12:22 -0500:
> Update of /projects/cvsroot/pvfs2/src/common/misc
> In directory parlweb1:/tmp/cvs-serv15660/src/common/misc
>
> Added Files:
> fsck-utils.c fsck-utils.h
> Log Message:
> missed adding these in recent commit of pvfs2-validate code.
[..]
> +/**
> + * Performs validity checking for the PVFS_TYPE_DIRECTORY directory entries
> + * Checks the following:
> + * - invalid characters in entry names
> + * - entry_names <= PVFS2_SEGMENT_MAX
> + * - warnings for characters that tend to confuse shells
> + * .
> + * \retval 0 on success
> + * \retval -PVFS_EWARNING for non critical warnings
> + * \retval -PVFS_error on failure
> + */
> +int PVFS_fsck_validate_dir_ent(
> + const struct PINT_fsck_options *fsck_options, /**< generic fsck options
> */
> + const char *filename) /**< Filename associated with handle
> */
> +{
> + int ret = 0;
> +
> + if (strlen(filename) > PVFS_SEGMENT_MAX)
> + {
> + gossip_err(
> + "Error: directory entry [%s] length is > PVFS_SEGMENT_MAX
> [%d]\n",
> + filename, PVFS_SEGMENT_MAX);
> + set_return_code(&ret, -PVFS_EINVAL);
> + }
> +
> + if (strspn(filename, "/") > 0)
> + {
> + gossip_err("Error: directory entry [%s] contains invalid / character
> \n",
> + filename);
> + set_return_code(&ret, -PVFS_EINVAL);
> + }
> +
> + if (strspn(filename, "\n") > 0)
> + {
> + gossip_err(
> + "WARNING: directory entry [%s] contains invalid new line
> character\n",
> + filename);
> + set_return_code(&ret, -PVFS_EWARNING);
> + }
> +
> + if (strspn(filename, "\r") > 0)
> + {
> + gossip_err(
> + "WARNING: directory entry [%s] contains invalid carriage
> return character\n",
> + filename);
> + set_return_code(&ret, -PVFS_EWARNING);
> + }
> +
> + if (strspn(filename, "|") > 0)
> + {
> + gossip_err(
> + "WARNING: directory entry [%s] contains invalid pipe
> character\n",
> + filename);
> + set_return_code(&ret, -PVFS_EWARNING);
> + }
> +
> + if (strspn(filename, "*") > 0)
> + {
> + gossip_err(
> + "WARNING: directory entry [%s] contains invalid #
> character\n",
> + filename);
> + set_return_code(&ret, -PVFS_EWARNING);
> + }
> +
> + if (strspn(filename, "?") > 0)
> + {
> + gossip_err(
> + "WARNING: directory entry [%s] contains invalid ?
> character\n",
> + filename);
> + set_return_code(&ret, -PVFS_EWARNING);
> + }
> +
> + return ret;
> +}
I'm not too happy about this part. Three reasons:
1. -PVFS_EWARNING: An error that is actually a warning? There's
lots of spots in this code that look like:
err = PVFS_do_something(..)
if (err < 0 && err != -PVFS_EWARNING)
die()
It seems icky to have to have every caller filter out the errors
that aren't _real_ errors.
2. There's nothing illegal about any of the characters above
except for '/' (and '\0'). Names "." and ".." have special
meaning but you handle that separately elsewhere.
3. You're rather selective about the "bad practice" characters.
If *?|\r\n are bad, why not \010 or \177 or ! or # or ' or any
of the other characters that need a shell escape? Seems rather
arbitrary.
Can we get rid of these warnings and the infrastructure for
handling the not-quite-an-error?
-- Pete
_______________________________________________
Pvfs2-developers mailing list
[email protected]
http://www.beowulf-underground.org/mailman/listinfo/pvfs2-developers
