Corrections to affs and sfs
PATCH 1/1 Corrections to affs and sfs
Hello,
I would like to submit the following patch:
* fs/affs.c: Return failure when directory iteration failed.
* fs/sfs.c: Return failure when directory iteration failed. Correct order in which btree nodes are read.
These changes are needed to boot AROS Research Operating System from SFS / FFS.
Best regards,
Krzysztof
affs.c +1 -1
@@ -381,7 +381,7 @@ fail:grub_free (node);grub_free (hashtable); -return 1 ; +return 0 ; }
sfs.c +4 -11
@@ -172,7 +172,8 @@return grub_errno; } -for (i =0 ; i <grub_be_to_cpu16 (tree->nodes); i++) +grub_uint16_t nodescount =grub_be_to_cpu16 (tree->nodes); +for (i = nodescount -1 ; i >=0 ; i--) {#define EXTNODE (tree, index) \
@@ -180,17 +181,9 @@ + (index) * (tree)->nodesize))/* Follow the tree down to the leaf level. */ -if ((grub_be_to_cpu32 (EXTNODE (tree, i)->key) >= block) +if ((grub_be_to_cpu32 (EXTNODE (tree, i)->key) <= block) && !tree->leaf) { - next =grub_be_to_cpu32 (EXTNODE (tree, i -1 )->data); -break ; - } - -/* In case the last node is reached just use that one, it is -the right match. */ -if (i +1 ==grub_be_to_cpu16 (tree->nodes) && !tree->leaf) - { next =grub_be_to_cpu32 (EXTNODE (tree, i)->data);break ; }
@@ -451,7 +444,7 @@ fail:grub_free (objc_data); -return 1 ; +return 0 ; }
Quoted 11 lines · Krzysztof Smiechowicz
Hello,
I would like to submit the following patch:
*
fs/affs.c: Return failure when directory iteration failed.*
fs/sfs.c: Return failure when directory iteration failed. Correct order in which btree nodes are read.These changes are needed to boot AROS Research Operating System from SFS / FFS.
Hi,
affs.c line 385
Quoted 11 lines
Index:
fs/affs.c
===================================================================affs.c +1
-1@@ -381,7 +381,7 @@ fail:grub_free (node);grub_free (hashtable); -return 1 ; +return 0 ; }
I checked in this hunk (and the equivalent sfs.c one)
sfs.c line 176
Quoted 11 lines
Index:
fs/sfs.c
===================================================================sfs.c +2
-1@@ -172,7 +172,8 @@return grub_errno; } -for (i =0 ; i <grub_be_to_cpu16 (tree->nodes); i++) +grub_uint16_t nodescount =grub_be_to_cpu16 (tree->nodes); +for (i = nodescount -1 ; i >=0 ; i--)
nodescount is only used once; why adding a variable?
Quoted 6 lines
sfs.c +1
-1/* Follow the tree down to the leaf level. */ -if ((grub_be_to_cpu32 (EXTNODE (tree, i)->key) >= block) +if ((grub_be_to_cpu32 (EXTNODE (tree, i)->key) <= block) && !tree->leaf)sfs.c +0
-1{ - next =grub_be_to_cpu32 (EXTNODE (tree, i -1 )->data);
sfs.c line 189
Quoted 10 lines
sfs.c +0
-7-break ; - } - -/* In case the last node is reached just use that one, it is -the right match. */ -if (i +1 ==grub_be_to_cpu16 (tree->nodes) && !tree->leaf) - { next =grub_be_to_cpu32 (EXTNODE (tree, i)->data);break ; }
I'm not familiar with our SFS code. Marco, if you have a minute could you review this part?
Signature · 5 lines
Robert Millan The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and how) you may access your data; but nobody's threatening your freedom: we still allow you to remove your data and not access it at all."
Hello,
Robert Millan pisze:
Quoted 8 lines
Quoted 8 lines
Index:
fs/sfs.c
===================================================================sfs.c +0
-0@@ -172,7 +172,8 @@return grub_errno; }
Quoted 5 lines
sfs.c line 176
Quoted 3 lines
sfs.c +2
-1-for (i =0 ; i <grub_be_to_cpu16 (tree->nodes); i++) +grub_uint16_t nodescount =grub_be_to_cpu16 (tree->nodes); +for (i = nodescount -1 ; i >=0 ; i--)nodescount is only used once; why adding a variable?
No particular reason. Feel free to correct it :)
Best regards,
Krzysztof
Robert Millan pisze:
Quoted 25 lines
sfs.c line 176
Quoted 11 lines
Index:
fs/sfs.c
===================================================================sfs.c +2
-1@@ -172,7 +172,8 @@return grub_errno; } -for (i =0 ; i <grub_be_to_cpu16 (tree->nodes); i++) +grub_uint16_t nodescount =grub_be_to_cpu16 (tree->nodes); +for (i = nodescount -1 ; i >=0 ; i--)nodescount is only used once; why adding a variable?
Quoted 11 lines
sfs.c +1
-1/* Follow the tree down to the leaf level. */ -if ((grub_be_to_cpu32 (EXTNODE (tree, i)->key) >= block) +if ((grub_be_to_cpu32 (EXTNODE (tree, i)->key) <= block) && !tree->leaf)sfs.c +0
-6{ - next =grub_be_to_cpu32 (EXTNODE (tree, i -1 )->data); -break ; - } - -/* In case the last node is reached just use that one, it is -the right match. */
Quoted 9 lines
sfs.c line 189
Quoted 5 lines
sfs.c +0
-2-if (i +1 ==grub_be_to_cpu16 (tree->nodes) && !tree->leaf) - { next =grub_be_to_cpu32 (EXTNODE (tree, i)->data);break ; }I'm not familiar with our SFS code. Marco, if you have a minute could you review this part?
Hello,
Any news about applying this patch? :)
Best regards,
Krzysztof
AROS Development Team
Quoted 37 lines · Krzysztof Smiechowicz
Robert Millan pisze:
Quoted 25 lines
sfs.c line 176
Quoted 11 lines
Index:
fs/sfs.c
===================================================================sfs.c +2
-1@@ -172,7 +172,8 @@return grub_errno; } -for (i =0 ; i <grub_be_to_cpu16 (tree->nodes); i++) +grub_uint16_t nodescount =grub_be_to_cpu16 (tree->nodes); +for (i = nodescount -1 ; i >=0 ; i--)nodescount is only used once; why adding a variable?
Quoted 11 lines
sfs.c +1
-1/* Follow the tree down to the leaf level. */ -if ((grub_be_to_cpu32 (EXTNODE (tree, i)->key) >= block) +if ((grub_be_to_cpu32 (EXTNODE (tree, i)->key) <= block) && !tree->leaf){sfs.c +0
-6- next =grub_be_to_cpu32 (EXTNODE (tree, i -1 )->data); -break ; - } - -/* In case the last node is reached just use that one, it is -the right match. */ Quoted 9 lines
Quoted 5 lines
sfs.c +0
-2-if (i +1 ==grub_be_to_cpu16 (tree->nodes) && !tree->leaf) - {next = grub_be_to_cpu32 (EXTNODE (tree, i)->data); break; }I'm not familiar with our SFS code. Marco, if you have a minute could you review this part?
Quoted 3 lines
Hello,
Any news about applying this patch? :)
Since Marco (who wrote that file) is not currently active, and nobody else seems to be able to test and/or proofread it, we could just trust that you know what you're doing.
Please, could you resend the patch without the unneeded 'nodescount' and a ChangeLog entry?
Signature · 5 lines
Robert Millan The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and how) you may access your data; but nobody's threatening your freedom: we still allow you to remove your data and not access it at all."
Quoted 4 lines
Hello,
Any news about applying this patch? :)
Hello,
I'm resubmitting the updated patch.
Changelog:
2009-02-11 Krzysztof Smiechowicz <[email protected]>
* fs/sfs.c (grub_sfs_read_extent) : Correction to traversing extent
b-tree
sfs.c +2 -10
@@ -172,7 +172,7 @@return grub_errno; } -for (i =0 ; i <grub_be_to_cpu16 (tree->nodes); i++) +for (i =grub_be_to_cpu16 (tree->nodes) -1 ; i >=0 ; i--) {#define EXTNODE (tree, index) \
@@ -180,17 +180,9 @@ + (index) * (tree)->nodesize))/* Follow the tree down to the leaf level. */ -if ((grub_be_to_cpu32 (EXTNODE (tree, i)->key) >= block) +if ((grub_be_to_cpu32 (EXTNODE (tree, i)->key) <= block) && !tree->leaf) { - next =grub_be_to_cpu32 (EXTNODE (tree, i -1 )->data); -break ; - } - -/* In case the last node is reached just use that one, it is -the right match. */ -if (i +1 ==grub_be_to_cpu16 (tree->nodes) && !tree->leaf) - { next =grub_be_to_cpu32 (EXTNODE (tree, i)->data);break ; }