lkml.org 
[lkml]   [2008]   [Mar]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [Pull] Some documentation patches
    Jan Engelhardt пишет:
    >
    > On Friday 2008-03-28 19:20, Jonathan Corbet wrote:
    >> commit 9756ccfda31b4c4544aa010aacf71b6672d668e8
    >> Date: Fri Mar 28 11:19:56 2008 -0600
    >>
    >> Add the seq_file documentation
    >
    > patch on top:
    >
    > - add const qualifiers
    > - remove void* casts
    > - use proper specifier (%Ld is not valid)
    >
    > Signed-off-by: Jan Engelhardt <jengelh@computergmbh.de>
    >
    > diff --git a/Documentation/filesystems/seq_file.txt
    > b/Documentation/filesystems/seq_file.txt
    > index 92975ee..cc6cdb9 100644
    > --- a/Documentation/filesystems/seq_file.txt
    > +++ b/Documentation/filesystems/seq_file.txt
    > @@ -107,8 +107,8 @@ complete. Here's the example version:
    >
    > static void *ct_seq_next(struct seq_file *s, void *v, loff_t *pos)
    > {
    > - loff_t *spos = (loff_t *) v;
    > - *pos = ++(*spos);
    > + loff_t *spos = v;
    > + *pos = ++*spos;

    Excuse me, what's the point in this change and the next one? IMO, removing
    the explicit type cast makes the code less obvious (AFAICT, this is a trendy
    word in LKML these days). Relying upon operator priorities instead of explicit
    operator grouping using parentheses can confuse people, too. Imagine a
    person looking at these lines: after the change, he or she will need to check
    the variable v type in the argument list, and consult the table of operator
    priorities in C if the person is in doubt about what the code does.

    Just my two cents...

    Dmitri

    > return spos;
    > }
    >
    > @@ -127,8 +127,8 @@ something goes wrong. The example module's show()
    > function is:
    >
    > static int ct_seq_show(struct seq_file *s, void *v)
    > {
    > - loff_t *spos = (loff_t *) v;
    > - seq_printf(s, "%Ld\n", *spos);
    > + loff_t *spos = v;
    > + seq_printf(s, "%lld\n", (long long)*spos);
    > return 0;
    > }
    >
    > @@ -136,7 +136,7 @@ We will look at seq_printf() in a moment. But first,
    > the definition of the
    > seq_file iterator is finished by creating a seq_operations structure with
    > the four functions we have just defined:
    >
    > - static struct seq_operations ct_seq_ops = {
    > + static const struct seq_operations ct_seq_ops = {
    > .start = ct_seq_start,
    > .next = ct_seq_next,
    > .stop = ct_seq_stop,
    > @@ -204,7 +204,7 @@ line, as in the example module:
    > static int ct_open(struct inode *inode, struct file *file)
    > {
    > return seq_open(file, &ct_seq_ops);
    > - };
    > + }
    >
    > Here, the call to seq_open() takes the seq_operations structure we created
    > before, and gets set up to iterate through the virtual file.
    > @@ -219,7 +219,7 @@ The other operations of interest - read(), llseek(),
    > and release() - are
    > all implemented by the seq_file code itself. So a virtual file's
    > file_operations structure will look like:
    >
    > - static struct file_operations ct_file_ops = {
    > + static const struct file_operations ct_file_ops = {
    > .owner = THIS_MODULE,
    > .open = ct_open,
    > .read = seq_read,
    > --
    > 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/
    >

    --
    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: 2008-03-31 16:35    [W:0.028 / U:123.148 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site