removed unneeded variable in signify

2014-02-27 Thread Fritjof Bornebsuch
Hi tech,

this diff removes an unneeded variable from signify.
The int is only used for a comparison in order to make sure the b64 encoding 
was right.

Regards,
Fritjof

Index: signify.c
===
RCS file: /cvs/src/usr.bin/signify/signify.c,v
retrieving revision 1.41
diff -u -p -u -r1.41 signify.c
--- signify.c   22 Jan 2014 21:11:03 -  1.41
+++ signify.c   27 Feb 2014 13:17:16 -
@@ -136,7 +136,6 @@ static size_t
 parseb64file(const char *filename, char *b64, void *buf, size_t len,
 char *comment)
 {
-   int rv;
char *commentend, *b64end;
 
commentend = strchr(b64, '\n');
@@ -154,8 +153,7 @@ parseb64file(const char *filename, char 
if (!b64end)
errx(1, missing new line after b64 in %s, filename);
*b64end = 0;
-   rv = b64_pton(commentend + 1, buf, len);
-   if (rv != len)
+   if (b64_pton(commentend + 1, buf, len) != len)
errx(1, invalid b64 encoding in %s, filename);
if (memcmp(buf, PKALG, 2))
errx(1, unsupported file %s, filename);



signify manpage example

2014-02-27 Thread Fritjof Bornebsuch
Hi tech,

I find the signify manpage regarding the examples a little bit strange.
The sign example shows not the relationship between the message that should be 
signed 
and the signature file.

So I changed the manpage a bit to be more clear.

Regards,
Fritjof

Index: signify.1
===
RCS file: /cvs/src/usr.bin/signify/signify.1,v
retrieving revision 1.22
diff -u -p -u -r1.22 signify.1
--- signify.1   17 Jan 2014 03:38:12 -  1.22
+++ signify.1   27 Feb 2014 13:58:05 -
@@ -126,10 +126,10 @@ Create a new keypair:
 .Dl $ signify -G -p newkey.pub -s newkey.sec
 .Pp
 Sign a file, specifying a signature name:
-.Dl $ signify -S -s key.sec -m message.txt -x msg.sig
+.Dl $ signify -S -s key.sec -m message.txt -x message.txt.sig
 .Pp
 Verify a signature, using the default signature name:
-.Dl $ signify -V -p key.pub -m generalsorders.txt
+.Dl $ signify -V -p key.pub -m message.txt
 .Pp
 Verify a release directory containing
 .Pa SHA256.sig



Re: trivial fix for pmemrange

2014-02-27 Thread Ariane van der Steldt

On 17/02/14 10:11, Kieran Devlin wrote:

the original implementation use identical logic for both ‘high’  ‘low’,
which will cause ‘high’  ‘low’ end up at same RB-tree node, instead of an 
expected interval.
and finally, break ‘for’ loop logic

luckily all these conditions never meet in practice.


Hmm, it may hit when you have a device that requires 2-4GB memory in a 
0-4GB memory range, which is not entirely unlikely... (just to give an 
example).



On Feb 17, 2014, at 5:14 PM, Mike Larkin mlar...@azathoth.net wrote:

Probably get a better response if you explained what this diff does
and/or fixes…


I have to agree with Mike here.  As someone who is rather familiar with 
the code, I still required a fair amount of time to figure out what the 
original code is doing and how the diff affects it.


Remember that not every developer is as familiar with the code as you 
are. :)



i just think this bug is a little too obvious


Most bugs are, once you find them.  Until that happens, they look 
perfectly valid.  I think the original body of code took something like 
30+ revisions...


Anyway, to respond to the question that Mike asked:

--- Intended behaviour ---

The function in question, essentially looks for a page satisfying the 
request condition:

[1] it must intersect with [low_addr - high_addr]
[2] it must be of a given memory type
It will return a pointer to any (arbitrary) range satisfying these 
conditions.


The implementation is based on lower-bound and upper-bound lookup in a 
tree.  It first clamps the range such that the upper (high) and lower 
(low) bounds are at or around [1]  (i.e. traversing low - high would 
cover the entirety of [1]).


As an optimization, if it finds a range that already satisfies the 
request, it quickly returns it (this is why the loops contain return 
statements).


As the last step, it traverses the entire range O(n log n) of pages in 
the range low-high, returning the first match.


--- The fix ---

Kieran correctly notes that the code does the lower bound lookup is 
incorrect.  Instead of walking downward, it walks upward, essentially 
performing the same traversal as the loop above (for variable 'high') 
and ending up at the same position.


The final loop therefore traverses an empty range.  Instead of using 
RB_RIGHT, RB_LEFT should be used to select ever lower addresses, up to 
falling out of the range.


--- Additional thoughts ---

I doubt that function actually does what it says in the comment, after 
the fix is applied.  I would recommend checking the logic in the last 
loop as well, since from reading it back, I think it may still exhibit 
false negatives.  I doubt it will trigger false positives though.


--
Ariane