Hi Matthias,
On Wed, May 02, 2007 at 12:33:29PM +0200, Matthias Koenig wrote:
>
> this patch adds libvolume_id support and removes the old fs detection code.
Thanks a lot! This is really important patch for the next release.
> Since I had to clean up the original patch I decided to remove the old fs
> detection code also in this patch.
> I hope this does not bloat the patch too much.
Well, I'm always more happy with bunch of small patches rather than with
one huge patch :-)
> const char *
> -mount_get_devname_by_uuid(const char *uuid) {
> +mount_get_devname_by_uuid(const char *uuid)
> +{
hmm... very useful change :-)
> --- /dev/null
> +++ util-linux-devel/mount/mount_udev.c
why not mount_volumeid.c ? (especially if there is
--with-libvolume_id)
> + value = calloc(1, VOLUME_ID_LABEL_SIZE);
> + if (!value)
> + return NULL;
Please, use routines from xmalloc.h (you can append the xcalloc).
> +
> + if (!spec)
> + return NULL;
> +
> + while (volume_id_ptr->token && strcmp(volume_id_ptr->token,token))
> + volume_id_ptr++;
> +
> + if (!volume_id_ptr->token) {
> + free(value);
> + value = NULL;
> + goto out;
> + }
> +
> + /* Quick exit if ID_FS_* variables are set */
> + if ((var = getenv(volume_id_ptr->env))) {
> + strncpy(value,var,VOLUME_ID_LABEL_SIZE - 1);
> + goto out;
> + }
Who need this ID_FS_* feature? That's strange from my point of view
and I'll feel better when this feature will be disabled for normal
(UID!=0) users (just in case we really need this feature).
> +void
> +mount_blkid_get_cache(void) {}
> +
> +void
> +mount_blkid_put_cache(void) {}
The API should be generic and independent on blkid or volumenid.
mount_volume_cache_init()
mount_volume_cache_deinit()
and also
git mv mount_blkid.h mount_volume.h.
(or something better -- I have a very limited imagination when I
think about new names :-)
> +const char *
> +mount_get_devname_by_uuid(const char *uuid) {
> + char *dev = NULL;
> +
> + if (!uuid)
> + return NULL;
> +
> + dev = malloc(19 + strlen(uuid));
> + strcpy(dev,"/dev/disk/by-uuid/");
> + strcat(dev,uuid);
> + }
> +
> + return dev;
> +}
> +
> +const char *
> +mount_get_devname_by_label(const char *label) {
> + char *dev = NULL;
> +
> + if (!label)
> + return NULL;
> +
> + dev = malloc(20 + strlen(label));
> + if (dev) {
> + strcpy(dev,"/dev/disk/by-label/");
> + strcat(dev,label);
> + }
> +
> + return dev;
> +}
19, 20 .. magic numbers? ;-)
#define DEV_BYLABEL_DIR "/dev/disk/by-label"
and sizeof() seems like a better solution.
Can't we canonicalize() the path to avoid /dev/disk/by-*
paths in /etc/mtab?
const char *
mount_get_devname_by_label(const char *label) {
char path[PATH_MAX];
if (!label)
return NULL;
snprintf(path, sizeof(path), DEV_BYLABEL_DIR "/%s", label);
return canonicalize(path);
}
> -#include "mount_by_label.h"
> +//#include "mount_by_label.h"
^^^
Please, always /* */ ...
Karel
--
Karel Zak <kzak@xxxxxxxxxx>
Red Hat Czech s.r.o.
Purkynova 99/71, 612 45 Brno, Czech Republic
Reg.id: CZ27690016
-
To unsubscribe from this list: send the line "unsubscribe util-linux-ng" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
