Hi Geyslan,
On Fri, Sep 27, 2013 at 02:59:12PM -0300, Geyslan Gregório Bem wrote:
> Hi Maintainers,
>
> I suppose the variables "highstale" and "lowstale" are being used despite
> not having been initialized.
>
> File: fs/xfs/xfs_dir2_node.c
> Function: xfs_dir2_leafn_add
>
> L491:
> > /*
> > * Insert the new entry, log everything.
> > */
> > lep = xfs_dir3_leaf_find_entry(&leafhdr, ents, index, compact, lowstale,
> > highstale, &lfloglow, &lfloghigh);
>
> The only place they are started up is within this condition:
>
> L480:
> > if (compact)
> > xfs_dir3_leaf_compact_x1(&leafhdr, ents, &index, &lowstale,
> > &highstale, &lfloglow, &lfloghigh);
>
> So, if it is not compact, both have garbage.
Thanks for the report. That sounds pretty bad. Lets see...
421 static int /* error */
422 xfs_dir2_leafn_add(
423 struct xfs_buf *bp, /* leaf buffer */
424 xfs_da_args_t *args, /* operation arguments */
425 int index) /* insertion pt for new entry */
426 {
...
476 /*
477 * Compact out all but one stale leaf entry. Leaves behind
478 * the entry closest to index.
479 */
480 if (compact)
481 xfs_dir3_leaf_compact_x1(&leafhdr, ents, &index, &lowstale,
482 &highstale, &lfloglow, &lfloghigh);
483 else if (leafhdr.stale) {
484 /*
485 * Set impossible logging indices for this case.
486 */
487 lfloglow = leafhdr.count;
488 lfloghigh = -1;
489 }
490
491 /*
492 * Insert the new entry, log everything.
493 */
494 lep = xfs_dir3_leaf_find_entry(&leafhdr, ents, index, compact, lowstale,
495 highstale, &lfloglow, &lfloghigh);
If compact is set at 481 we pass the addresses of highstale and lowstale to
xfs_dir3_leaf_compact_x1, which passes them to xfs_dir3_leaf_find_stale, which
makes assignments to both variables unconditionally. Later at 494 we pass
compact, lowstale, and highstale to xfs_dir3_leaf_find_entry.
So we're ok if compact is set...
555 struct xfs_dir2_leaf_entry *
556 xfs_dir3_leaf_find_entry(
557 struct xfs_dir3_icleaf_hdr *leafhdr,
558 struct xfs_dir2_leaf_entry *ents,
559 int index, /* leaf table position */
560 int compact, /* need to compact leaves */
561 int lowstale, /* index of prev stale leaf */
562 int highstale, /* index of next stale leaf */
563 int *lfloglow, /* low leaf logging index */
564 int *lfloghigh) /* high leaf logging index */
565 {
...
587 /*
588 * There are stale entries.
589 *
590 * We will use one of them for the new entry. It's probably not at
591 * the right location, so we'll have to shift some up or down first.
592 *
593 * If we didn't compact before, we need to find the nearest stale
594 * entries before and after our insertion point.
595 */
596 if (compact == 0)
597 xfs_dir3_leaf_find_stale(leafhdr, ents, index,
598 &lowstale, &highstale);
In xfs_dir3_leaf_find_entry, it looks like if compact is not set, we will pass
the addresses of lowstale and highstale to xfs_dir3_leaf_find_stale which
appears to make assignments to them unconditionally. It looks like
xfs_dir3_leaf_find_entry doesn't read from lowstale and highstale until after
598. I think this should take care of the !compact case too. Do you agree?
Thanks much,
Ben
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
