lkml.org 
[lkml]   [2006]   [Sep]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 14/16] GFS2: The DLM interface module

>+/* make_strname - convert GFS lock numbers to a string */
>+
>+static inline void make_strname(struct lm_lockname *lockname,
>+ struct gdlm_strname *str)
>+{
>+ sprintf(str->name, "%8x%16llx", lockname->ln_type,
>+ (unsigned long long)lockname->ln_number);

Is this format specifier safe enough? "%08x%016llx" perhaps?

Imagine (if this happens at all):

ln_type = 1; ln_number = 16;
%8x = "1", "%16llx" = "10", giving us "110"

ln_type = 17; ln_number = 0;
%8x = "11", "%16llx" = "0", giving us "110".

Whoops, name clash.


>+int gdlm_get_lock(lm_lockspace_t *lockspace, struct lm_lockname *name,
>+ lm_lock_t **lockp)
>+{
>+ struct gdlm_lock *lp;
>+ int error;
>+
>+ error = gdlm_create_lp((struct gdlm_ls *) lockspace, name, &lp);
>+
>+ *lockp = (lm_lock_t *) lp;

This cast is alright in itself. Considering however that lm_lock_t is
currently typedef'ed to void, it looks a little different. (One _could_ get rid
of it, but better not while it is called lm_lock_t. Leave as-is for now.)

>+static wait_queue_head_t send_wq;
>+static wait_queue_head_t recv_wq;
>+
>+struct plock_op {
>+ struct list_head list;
>+ int done;
>+ struct gdlm_plock_info info;
>+};
>+
>+static inline void set_version(struct gdlm_plock_info *info)
>+{
>+ info->version[0] = GDLM_PLOCK_VERSION_MAJOR;
>+ info->version[1] = GDLM_PLOCK_VERSION_MINOR;
>+ info->version[2] = GDLM_PLOCK_VERSION_PATCH;
>+}
>+
>+static int check_version(struct gdlm_plock_info *info)
>+{
>+ if ((GDLM_PLOCK_VERSION_MAJOR != info->version[0]) ||
>+ (GDLM_PLOCK_VERSION_MINOR < info->version[1])) {
>+ log_error("plock device version mismatch: "
>+ "kernel (%u.%u.%u), user (%u.%u.%u)",
>+ GDLM_PLOCK_VERSION_MAJOR,
>+ GDLM_PLOCK_VERSION_MINOR,
>+ GDLM_PLOCK_VERSION_PATCH,
>+ info->version[0],
>+ info->version[1],
>+ info->version[2]);
>+ return -EINVAL;
>+ }
>+ return 0;
>+}
>+
>+static void send_op(struct plock_op *op)
>+{
>+ set_version(&op->info);
>+ INIT_LIST_HEAD(&op->list);
>+ spin_lock(&ops_lock);
>+ list_add_tail(&op->list, &send_list);
>+ spin_unlock(&ops_lock);
>+ wake_up(&send_wq);
>+}
>+
>+int gdlm_plock(lm_lockspace_t *lockspace, struct lm_lockname *name,
>+ struct file *file, int cmd, struct file_lock *fl)
>+{
>+ struct gdlm_ls *ls = (struct gdlm_ls *) lockspace;
>+ struct plock_op *op;
>+ int rv;
>+
>+ op = kzalloc(sizeof(*op), GFP_KERNEL);
>+ if (!op)
>+ return -ENOMEM;
>+
>+ op->info.optype = GDLM_PLOCK_OP_LOCK;
>+ op->info.pid = fl->fl_pid;
>+ op->info.ex = (fl->fl_type == F_WRLCK);
>+ op->info.wait = IS_SETLKW(cmd);
>+ op->info.fsid = ls->id;
>+ op->info.number = name->ln_number;
>+ op->info.start = fl->fl_start;
>+ op->info.end = fl->fl_end;
>+ op->info.owner = (__u64)(long) fl->fl_owner;

Can't op->info.owner be a 'struct fowner *'? Is op->info.owner shared over the
network?

>+static ssize_t block_show(struct gdlm_ls *ls, char *buf)
>+{
>+ ssize_t ret;
>+ int val = 0;
>+
>+ if (test_bit(DFL_BLOCK_LOCKS, &ls->flags))
>+ val = 1;
>+ ret = sprintf(buf, "%d\n", val);

Safe enough - @buf big enough?

>+ if (val == 1)
>+ set_bit(DFL_BLOCK_LOCKS, &ls->flags);
>+ else if (val == 0) {
>+ clear_bit(DFL_BLOCK_LOCKS, &ls->flags);
>+ gdlm_submit_delayed(ls);
>+ } else
>+ ret = -EINVAL;

Ingo surely wants you to {} it.

>+static ssize_t id_show(struct gdlm_ls *ls, char *buf)
>+{
>+ return sprintf(buf, "%u\n", ls->id);
>+}
>+
>+static ssize_t jid_show(struct gdlm_ls *ls, char *buf)
>+{
>+ return sprintf(buf, "%d\n", ls->jid);
>+}
>+
>+static ssize_t first_show(struct gdlm_ls *ls, char *buf)
>+{
>+ return sprintf(buf, "%d\n", ls->first);
>+}
>+
>+static ssize_t first_done_show(struct gdlm_ls *ls, char *buf)
>+{
>+ return sprintf(buf, "%d\n", ls->first_done);
>+}
>+
>+static ssize_t recover_show(struct gdlm_ls *ls, char *buf)
>+{
>+ return sprintf(buf, "%d\n", ls->recover_jid);
>+}

Big enough?

>+static ssize_t recover_done_show(struct gdlm_ls *ls, char *buf)
>+{
>+ return sprintf(buf, "%d\n", ls->recover_jid_done);
>+}
>+
>+static ssize_t recover_status_show(struct gdlm_ls *ls, char *buf)
>+{
>+ return sprintf(buf, "%d\n", ls->recover_jid_status);
>+}





Jan Engelhardt
--
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2006-09-05 14:11    [W:0.100 / U:9.908 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site