[issue1863] Implement 'hammer volume-list' subcommand
Aggelos Economopoulos
aoiko at cc.ece.ntua.gr
Tue Oct 19 14:23:52 PDT 2010
On 10/19/2010 10:59 PM, Stathis Kamperis (via DragonFly issue tracker)
wrote:
>
> Stathis Kamperis <ekamperi at gmail.com> added the comment:
>
> New patch, same url:
> http://stathisk.ath.cx/0001-HAMMER-Implement-volume-list-command.patch
Can you explain why you use the 0001- prefix if you're going to use the
same url over and over again? It's just extra pain for us to rename your
patches, which is something I did, so now I can do the following, in
order to only review your changes relative to the last patch. Comments
inline.
$ diff -u 000{1,2}-HAMMER-Implement-volume-list-command.patch
-+ int i, cnt;
++ int i, cnt, len;
+
+ for (i = 0, cnt = 0; i < HAMMER_MAX_VOLUMES && cnt < ioc->nvols;
i++) {
+ volume = hammer_get_volume(hmp, i, &error);
@@ -213,8 +213,13 @@
+ }
+ KKASSERT(volume != NULL && error == 0);
+
++ len = strlen(volume->vol_name) + 1;
++ if (len > MAXPATHLEN) {
++ len = MAXPATHLEN;
++ volume->vol_name[len - 1] = '\0';
++ }
+ error = copyout(volume->vol_name,
ioc->vols[cnt].device_name,
-+ MAXPATHLEN);
++ len);
Looks good, although you should probably just KKASSERT(len <
MAXPATHLEN). Please commit.
Aggelos
More information about the Bugs
mailing list