The GNU Hurd - Patches: patch #6088, Entropy Patch
You are not allowed to post comments on this tracker with your current authentication level.
patch #6088: Entropy Patch
Submitter: | None | ||
Submitted: | Thu 12 Jul 2007 07:09:52 PM UTC | ||
Category: | None | Priority: | 5 - Normal |
Status: | None | Privacy: | Public |
Assigned to: | None | Originator Email: | -email is unavailable- |
Open/Closed: | Open | Planned Release: | None |
Wiki-like text discussion box: |
|
Fri 18 Jul 2008 01:53:37 PM UTC, comment #1: |
Michael Banck <mbanck> |
Thu 12 Jul 2007 07:09:52 PM UTC, original submission:
I'm just submitting the patch here, it's been wrapped up all into a single file. |
Anonymous |
Attached Files
file #16141: entropy_patch.diff added by mbanck (29KiB - text/x-patch - last known version of patch from 2007/08/28)
file #13316: entropy_patch.diff added by None (21KiB - application/octet-stream)
Depends on the following items: None found
Items that depend on this one: None found
Carbon-Copy List
There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.
Follow 2 latest changes.
Date | Changed by | Updated Field | Previous Value | => | Replaced by |
---|---|---|---|---|---|
2008-07-18 | mbanck | Attached File | - | Added entropy_patch.diff, #16141 | |
2007-07-12 | None | Attached File | - | Added entropy_patch.diff, #13316 |
Powered by Savane 3.13-02a9.
Corresponding source code
Comments on the 2007/08/28 version of the patch by Samuel Thibault:
This is bogus: sizeof(ior) gives the size of the pointer, not the size
of the pointed data.
Particularly useless modification :)
Shouldn't mm_deltaX and mm_deltaY rather get shifted left and ORed?
Again, sizeof(skb) gives you the size of the pointer, not the size of
the skb structure... Also, putting the content of the skb in the entropy
pool won't put the network data in the pool, you need to put skb->data,
skb->len for that.
...
Isn't entropyinit() supposed to be called only once? If yes, then this
is useless: static data is always already initialized to zero by
default.
I'd rather see this chick inside entropyinit() itself, but even better,
you should manage to have entropyinit() called just once early enough
during the startup instead of calling it all the time.
Shouldn't that be if (amt >= len), since that case will deplete the pool
too?
Instead of a global variable which you have to keep up to date, can't
you just read entropy_amount instead of reading is_entropy_empty?
Bogus: you're putting data twice at ior->io_data. You need to add cnt
for the second memcpy.
Same as above.
This time it's correct. Shouldn't both functions share such code instead
of duplicating potential bugs?
Please take care of spelling :) (there are a few other such typos in
comments in several places)
Really please take care :)
missing indentation.
Same remark as for is_entropy_empty: I don't see the point of keeping
a separate coherency-bug-prone global state variable rather than just
reading entropy_amount instead of reading use_entropic_average.
This total, however, could be cached so that you don't have to recompute
it.
IIRC I already said that this should really not be a while loop: this
function only provides one character of entropy, so it's only useful
to wake one reader.
We should probably have an architecture-dependant macro for more precise
timestamps: starting from the pentium we could use rdtsc for instance.
In such case we could announce a yet better entropy quality.
Default values usually belong to configure scripts (so that --help can
give it, for instance), so these lines should go there.