Re: CVS commit: src/sys/ufs/ffs

[Maxime Villard]

Le 14/02/2015 20:21, David Holland a écrit :
 On Sat, Feb 14, 2015 at 08:07:39AM +, Maxime Villard wrote:
   Modified Files:
  src/sys/ufs/ffs: ffs_appleufs.c
   
   Log Message:
   ffs_appleufs_validate():
- remove superfluous printfs
- ensure ul_namelen!=0, otherwise the kernel accesses ul_name[-1] and
  overwrites the previous field in the structure.
 
 Did you test this?

It is the only change I didn't test. This change was superficial anyway.

Verily, I didn't understand what this function was supposed to do: it is never
called with n!=NULL, and the misleading comment plus the negative index
convinced me it was just dead.

 It is almost certain that this bit:
 
 *n = *o;
 -   n-ul_checksum = 0;
 n-ul_checksum = ffs_appleufs_cksum(n);
 
 breaks it.

Yes, you are right. I thought it was another misleading instruction.

 Also, I think you might want to keep the print when the
 checksum is wrong.
 

If we keep this print, then we keep a lot of other prints. So we don't keep it.

Thanks!

re: CVS commit: src/sys/ufs/ffs

[matthew green]

  Also, I think you might want to keep the print when the
  checksum is wrong.
 
 If we keep this print, then we keep a lot of other prints. So we don't keep 
 it.

i don't follow.

usually these messages are the only real indication of what is
actually wrong, and they aren't log spew problems.

what are you trying to solve by removing them?  and what is this
list of lot of other prints?  each one should be considered on
its own pros/cons.


.mrg.

Re: CVS commit: src/sys/ufs/ffs

[Christos Zoulas]

In article 26607.1424011...@splode.eterna.com.au,
matthew green  m...@eterna.com.au wrote:

  Also, I think you might want to keep the print when the
  checksum is wrong.
 
 If we keep this print, then we keep a lot of other prints. So we don't
keep it.

i don't follow.

usually these messages are the only real indication of what is
actually wrong, and they aren't log spew problems.

what are you trying to solve by removing them?  and what is this
list of lot of other prints?  each one should be considered on
its own pros/cons.

I agree, the printfs are VERY useful. They are the only indication
to what's wrong when mount returns EINVAL. If you are mounting a filesystem
and it does not work, you usually end up putting them back :-)

christos