Skip to contentSkip to conversation

fault_in_multipages_readable() throws set-but-unused error

PATCH Applied Dave Chinner 3 participants 5 messages Sep 25, 2016 – Sep 26, 2016 Page cache

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 fault_in_multipages_readable() throws set-but-unused error

From: Dave Chinner <[email protected]>

When building XFS with -Werror, it now fails with:

./include/linux/pagemap.h: In function ¿fault_in_multipages_readable¿:
./include/linux/pagemap.h:602:16: error: variable ¿c¿ set but not used [-Werror=unused-but-set-variable]

  volatile char c;
                ^

This is a regression caused by commit e23d415 ("fix fault_in_multipages_...() on architectures with no-op access_ok()"). Fix it by re-adding the "(void)c" trick taht was previously used to make the compiler think the variable is used.

---
 include/linux/pagemap.h | 1 +
 1 file changed, 1 insertion(+)
include/linux/pagemap.h +1 -0
index 7e3d537..01e8443 100644
@@ -620,6 +620,7 @@ static inline int fault_in_multipages_readable(const char __user *uaddr,
 		return __get_user(c, end);
 	}
 
+	(void)c;
 	return 0;
 }
 
Signature · 1 line
2.8.0.rc3
#1 lore raw

Thanks, applied.

I did happen to notice:

Quoted 3 lines · Dave Chinner

./include/linux/pagemap.h: In function ¿fault_in_multipages_readable¿:
./include/linux/pagemap.h:602:16: error: variable ¿c¿ set but not used [-Werror=unused-but-set-variable]

You have some nasty unicode corruption. The email is marked as being

Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

but those are not the right unicode characters. I think gcc actually uses back-tick and tick (which is ugly as hell in many fonts since they aren't generally necessarily symmetric, but oh well). So some cut-and-paste path of yours corrupted the utf8.

Obviously not a big deal, but you might want to look at your character set setting in your mailer or other environment. Perhaps some odd non-utf8 editor environment or something?

I also noticed:

Quoted 2 lines

This is a regression caused by commit e23d415 ("fix fault_in_multipages_...() on architectures with no-op access_ok()").

I'd suggest doing

     git config --global core.abbrev 12

because the default git commit shortening value of 7 is practically too short for the kernel these days. What can I say? I was naïve, and based my original abbreviation decision on the BitKeeper numbers, where we were just about to hit 64k commits. So seven hex digits seemed plenty. And now, ten years later, I just look stupid.

Anyway - while git will always pick a shortname that is unique (ie it will add characters until it is unique), it's unique only at that moment in time. And practically speaking seven hex digits can easily have collisions in the hear future (ie it already happens with many commits).

So telling git "give me a minimum of 12 hex digits in hash abbreviations" should be fairly safe for a while.

I guess I should ping Junio and see if we can just change the default, but for smaller projects the default abbreviation is likely still reasonable. So I've been just mentioning that simple "set core.abbrev to 12" to kernel people..

Linus

#2 lore raw
Quoted 6 lines · Linus Torvalds

Thanks, applied.

I did happen to notice:

Quoted 1 line · Dave Chinner
Quoted 7 lines
Quoted 2 lines

./include/linux/pagemap.h: In function ¿fault_in_multipages_readable¿:
./include/linux/pagemap.h:602:16: error: variable ¿c¿ set but not used [-Werror=unused-but-set-variable]

You have some nasty unicode corruption. The email is marked as being

Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

It's whatever git-send-email spat out. I was under the impression it encodes like that whenever it sees a utf-8 character in a commit...

Quoted 4 lines

but those are not the right unicode characters. I think gcc actually uses back-tick and tick (which is ugly as hell in many fonts since they aren't generally necessarily symmetric, but oh well). So some cut-and-paste path of yours corrupted the utf8.

Yup, normally I remember to fix that up when composing the patch - ever since GNU went to those fucked up "grammatically correct" non-ascii quotes it's caused problems with pasting error messages into ascii-only contexts.

Quoted 3 lines

Obviously not a big deal, but you might want to look at your character set setting in your mailer or other environment. Perhaps some odd non-utf8 editor environment or something?

I turned off utf-8 support in vim on the machine I write all my code on - I got sick of stupid stray marks in my code, digraphs being composed when I just want to replace a single character, git sending patches in utf-8 encoding because I copied a SoB with a utf-8 character in the name, etc....

Quoted 8 lines

I also noticed:

Quoted 2 lines

This is a regression caused by commit e23d415 ("fix fault_in_multipages_...() on architectures with no-op access_ok()").

I'd suggest doing

     git config --global core.abbrev 12

Ok.

Quoted 2 lines

because the default git commit shortening value of 7 is practically too short for the kernel these days.

7 characters, 12 characters, whatever. Neither make any sense in commit messages by themselves without the short description that goes along with the hash.....

Cheers,

Dave.

Signature · 2 lines
Dave Chinner
[email protected]
#3 lore raw
Quoted 7 lines · Dave Chinner
Quoted 4 lines

You have some nasty unicode corruption. The email is marked as being

Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

It's whatever git-send-email spat out. I was under the impression it encodes like that whenever it sees a utf-8 character in a commit...

Yes. But it assumes that the commit text was in UTF-8 too. So I suspect it happened as you were writing the commit message:

Quoted 2 lines

I turned off utf-8 support in vim on the machine I write all my code on

I guess that's probably it. You probably had vim try to convert it to latin1 or something, and the odd utf-8 tick/back-tick thing doesn't do so cleanly, so..

Quoted 4 lines

- I got sick of stupid stray marks in my code, digraphs being composed when I just want to replace a single character, git sending patches in utf-8 encoding because I copied a SoB with a utf-8 character in the name, etc....

Hmm. Generally, the only place we should have non-us-ascii tensd to be exactly those names. But a sane editor should "just work" and not do odd crazy digraph crap or things like that. But I don't use vim, so I don't know what the magic incantation for sanity there is.

Quoted 3 lines

7 characters, 12 characters, whatever. Neither make any sense in commit messages by themselves without the short description that goes along with the hash.....

No, I agree, the right format for describing a commit tends to be along the lines of

.. commit %h ("%s") ..

exactly like you did. It's just that with 12 characters in the hex format, people will still be able to cut-and-paste the hash into git to get the full commit details even a year from now. With just 7 hex digits, you may well end up in the situation where you do

    git show e23d415

and git says

error: short SHA1 e23d415 is ambiguous.
fatal: ambiguous argument 'e23d415': unknown revision or path not
in the working tree.

(it's not ambiguous today, but in a year or two it quite possibly will be).

It's not impossible to figure out the different possible ambiguous commits, but it's just inconvenient.

Right now, of the roughly 618,000 commits in the mainline tree, about 98% can be uniquely represented with 7 hex digits. But about 2% o commits need eight or more hex digits to be unique (the git ID space includes all the tree and blob objects too, so the uniqueness is not just "unique within commits", but any kernel git object).

Linus

#4 lore raw

Hi all,

Quoted 8 lines · On Sun, 25 Sep 2016 20:06:19 -0700 Linus Torvalds…

exactly like you did. It's just that with 12 characters in the hex format, people will still be able to cut-and-paste the hash into git to get the full commit details even a year from now. With just 7 hex digits, you may well end up in the situation where you do

    git show e23d415
Quoted 7 lines

and git says

error: short SHA1 e23d415 is ambiguous.
fatal: ambiguous argument 'e23d415': unknown revision or path not
in the working tree.

(it's not ambiguous today, but in a year or two it quite possibly will be).

I have already had over the years a few instances of 7 digit commit hashes that are unique in Linus' tree but not in mine (which is a bit of a pain if you are trying to look for context).

Signature · 2 lines
Cheers,
Stephen Rothwell