Re: [issue1863] Implement 'hammer volume-list' subcommand

[Aggelos Economopoulos <aoiko@xxxxxxxxxxxxxx>]

On 10/19/2010 10:59 PM, Stathis Kamperis (via DragonFly issue tracker)
wrote:
> 
> Stathis Kamperis <ekamperi@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