[Pull] Some documentation patches

[Pull] Some documentation patches

[Jonathan Corbet]

I've noticed that getting documentation patches merged seems to be a
slower and more uncertain process than it was a while back.  So I
figured I'd try to be one of the cool folks with their own git tree and
see if that works better.  Linus, if you agree, could you please pull:

  git://git.lwn.net/linux-2.6.git docs

To get the following:

Jonathan Corbet (3):
      Add the seq_file documentation
      Fill out information on patch tags in SubmittingPatches
      Add a comment discouraging use of in_atomic()

 Documentation/SubmittingPatches        |   54 ++++++-
 Documentation/filesystems/00-INDEX     |    2 +
 Documentation/filesystems/seq_file.txt |  283 ++++++++++++++++++++++++++++++++
 include/linux/hardirq.h                |    8 +
 4 files changed, 344 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/filesystems/seq_file.txt

These changes are (1) an updated version of the seq_file document first
posted in 2003, (2) the much-reviewed patch tags documentation, and
(3) a comment warning developers that in_atomic() doesn't mean what they
think it means.  No code changes.

If this works out, and nobody objects, I'll try to run this tree into
the future as a collection point for documentation patches which don't
have a more obvious tree to travel through.

Thanks,

jon

Re: [Pull] Some documentation patches

[Jan Engelhardt]

On Friday 2008-03-28 19:20, Jonathan Corbet wrote:
>
>  git://git.lwn.net/linux-2.6.git docs

$ git remote add doc git://git.lwn.net/linux-2.6.git

$ git-fetch doc
fatal: Unable to look up git.lwn.net (port 9418) (Name or service not known)

Re: [Pull] Some documentation patches

[Linus Torvalds]

On Fri, 28 Mar 2008, Jonathan Corbet wrote:
>
>   git://git.lwn.net/linux-2.6.git docs

That seems to be a pretty recent DNS entry that hasn't percolated out yet. 
I get "git.lwn.net: NXDOMAIN" to my nslookup query (and git itself 
obviously says "Name or service not known").

I decided to try if it was the same machine as lwn.net, but it isn't.

So you'll need to resend this when it has percolated or send me an IP 
address ;)

		Linus

Re: [Pull] Some documentation patches

[Jonathan Corbet]

Linus Torvalds <torvalds@linux-foundation.org> wrote:

> That seems to be a pretty recent DNS entry that hasn't percolated out yet. 
> I get "git.lwn.net: NXDOMAIN" to my nslookup query (and git itself 
> obviously says "Name or service not known").
> 
> So you'll need to resend this when it has percolated or send me an IP 
> address ;)

Hmph.  It worked from the machines *I* tried it on...:(

Until the name gets out, pulling:

	git://vena.lwn.net/linux-2.6.git docs

will work.  That name has been out there for eight years or so, should
have percolated by now...

Sorry for the trouble,

jon

Re: [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;
  	        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,

Re: [Pull] Some documentation patches

[Jonathan Corbet]

Jan Engelhardt <jengelh@computergmbh.de> wrote:

> patch on top:
> 
>  - add const qualifiers
>  - remove void* casts
>  - use proper specifier (%Ld is not valid)

Looks good, I applied it.

Thanks,

jon

Re: [Pull] Some documentation patches

[Dmitri Vorobiev]

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/
> 

Re: [Pull] Some documentation patches

[Jan Engelhardt]

On Monday 2008-03-31 16:31, Dmitri Vorobiev wrote:
>> @@ -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.

Why should it be less obvious? Exactly left to the (loff_t *) you
already have loff_t *, so you know where you are going from v,
whose type is also obviously visible right in the line above.
Casts can hide errors. The cast is redundant, it does not
cover up a compiler warning. Even so, casts can hide programming
errors, as in http://jengelh.hopto.org/2007/0616-nocast.php .

> Relying upon operator priorities instead of explicit
> operator grouping using parentheses can confuse people, too.

In case of *a++ I would agree -- you would need to know whether * or
++ has the higher precedence (hint: ++ has). But in ++*a it does not
matter how much precedence either has, it always means the same.
Just like an arithmetic expression "x*-5".

Re: [Pull] Some documentation patches

[Will Newton]

On Fri, Mar 28, 2008 at 6:20 PM, Jonathan Corbet <corbet@lwn.net> wrote:
> I've noticed that getting documentation patches merged seems to be a
>  slower and more uncertain process than it was a while back.  So I
>  figured I'd try to be one of the cool folks with their own git tree and
>  see if that works better.  Linus, if you agree, could you please pull:

A small patch for the highres timers documentation:

Signed-off-by: Will newton <will.newton@gmail.com>

---
 Documentation/hrtimers/highres.txt |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Documentation/hrtimers/highres.txt
b/Documentation/hrtimers/highres.txt
index ce0e9a9..a73ecf5 100644
--- a/Documentation/hrtimers/highres.txt
+++ b/Documentation/hrtimers/highres.txt
@@ -98,7 +98,7 @@ System-level global event devices are used for the
Linux periodic tick. Per-CPU
 event devices are used to provide local CPU functionality such as process
 accounting, profiling, and high resolution timers.

-The management layer assignes one or more of the folliwing functions to a clock
+The management layer assigns one or more of the following functions to a clock
 event device:
       - system global periodic tick (jiffies update)
       - cpu local update_process_times
-- 
1.5.4.3

Re: [Pull] Some documentation patches

[Randy Dunlap]

Jonathan Corbet wrote:
> I've noticed that getting documentation patches merged seems to be a
> slower and more uncertain process than it was a while back.  So I
> figured I'd try to be one of the cool folks with their own git tree and
> see if that works better.  Linus, if you agree, could you please pull:

A lot of the time it's just a matter of the "merge window" for
non-critical patches.  OTOH, doc patches could be merged at just about
any time IMO.

>   git://git.lwn.net/linux-2.6.git docs
> 
> To get the following:
> 
> Jonathan Corbet (3):
>       Add the seq_file documentation
>       Fill out information on patch tags in SubmittingPatches
>       Add a comment discouraging use of in_atomic()
> 
>  Documentation/SubmittingPatches        |   54 ++++++-
>  Documentation/filesystems/00-INDEX     |    2 +
>  Documentation/filesystems/seq_file.txt |  283 ++++++++++++++++++++++++++++++++
>  include/linux/hardirq.h                |    8 +
>  4 files changed, 344 insertions(+), 3 deletions(-)
>  create mode 100644 Documentation/filesystems/seq_file.txt
> 
> These changes are (1) an updated version of the seq_file document first
> posted in 2003, (2) the much-reviewed patch tags documentation, and
> (3) a comment warning developers that in_atomic() doesn't mean what they
> think it means.  No code changes.
> 
> If this works out, and nobody objects, I'll try to run this tree into
> the future as a collection point for documentation patches which don't
> have a more obvious tree to travel through.

Getting doc patches merged can be slow sometimes (slower than needed),
but I'm still having success at it.

-- 
~Randy