[Linux-cluster] Re: GFS

Pekka J Enberg penberg at cs.helsinki.fi
Mon Aug 8 10:00:43 UTC 2005


David Teigland writes:
> > > +static int ea_set_i(struct gfs2_inode *ip, struct gfs2_ea_request *er,
> > > +                 struct gfs2_ea_location *el)
> > > +{
> > > +     {
> > > +             struct ea_set es;
> > > +             int error;
> > > +
> > > +             memset(&es, 0, sizeof(struct ea_set));
> > > +             es.es_er = er;
> > > +             es.es_el = el;
> > > +
> > > +             error = ea_foreach(ip, ea_set_simple, &es);
> > > +             if (error > 0)
> > > +                     return 0;
> > > +             if (error)
> > > +                     return error;
> > > +     }
> > > +     {
> > > +             unsigned int blks = 2;
> > > +             if (!(ip->i_di.di_flags & GFS2_DIF_EA_INDIRECT))
> > > +                     blks++;
> > > +             if (GFS2_EAREQ_SIZE_STUFFED(er) > ip->i_sbd->sd_jbsize)
> > > +                     blks += DIV_RU(er->er_data_len,
> > > +                                    ip->i_sbd->sd_jbsize);
> > > +
> > > +             return ea_alloc_skeleton(ip, er, blks, ea_set_block, el);
> > > +     }
> > 
> > Please drop the extra braces. 
> 
> Here and elsewhere we try to keep unused stuff off the stack.  Are you
> suggesting that we're being overly cautious, or do you just dislike the
> way it looks?

The extra braces hurt readability. Please drop them or make them proper 
functions instead. 

And yes, I think you're hiding potential stack usage problems here. Small 
unused stuff on the stack don't matter and large ones should probably be 
kmalloc() anyway. 

               Pekka 




More information about the Linux-cluster mailing list