[PATCH] NULLify ->raw after jffs2_mark_node_obsolete().

[Joakim Tjernlund]

On Wed, 2007-10-31 at 10:41 -0400, David Woodhouse wrote:
> On Tue, 2007-10-30 at 18:39 +0100, Joakim Tjernlund wrote:
> > --- a/fs/jffs2/nodelist.c
> > +++ b/fs/jffs2/nodelist.c
> > @@ -34,13 +34,19 @@ void jffs2_add_fd_to_list(struct jffs2_sb_info *c, struct jffs2_full_dirent *new
> >                         if (new->version < (*prev)->version) {
> >                                 dbg_dentlist("Eep! Marking new dirent node is obsolete, old is \"%s\", ino #%u\n",
> >                                         (*prev)->name, (*prev)->ino);
> > -                               jffs2_mark_node_obsolete(c, new->raw);
> > +                               if (new->raw) {
> > +                                       jffs2_mark_node_obsolete(c, new->raw);
> > +                                       new->raw = NULL;
> > +                               }
> >                                 jffs2_free_full_dirent(new);
> >                         } else {
> 
> 
> We're never going to be adding a _new_ node where fd->raw is NULL. This
> test is superfluous.

Well, that whole if stmt seems superfluous and jffs2_mark_node_obsolete
will complain if called with a NULL ptr so I guess it can go.

> 
> >                                 dbg_dentlist("marking old dirent \"%s\", ino #%u bsolete\n",
> >                                         (*prev)->name, (*prev)->ino);
> >                                 new->next = (*prev)->next;
> > -                               jffs2_mark_node_obsolete(c, ((*prev)->raw));
> > +                               if ((*prev)->raw) { /* Can be true during mount if a is unmarked in flash */
> > +                                       jffs2_mark_node_obsolete(c, ((*prev)->raw));
> > +                                       (*prev)->raw = NULL;
> > +                               }
> >                                 jffs2_free_full_dirent(*prev);
> >                                 *prev = new;
> >                         }
> 
> This one makes sense though, and is a necessary part of your first
> patch. After your first patch, we're going to be leaving full_dirents in
> the list which have NULL fd->raw -- and we mustn't crash when we later
> obsolete those dirents (by creating a new file with the same name).

Yes, that one bit me too but during mount when I had a node that was
obsolete but not marked so on flash.

> 
> > @@ -104,6 +110,7 @@ static void jffs2_obsolete_node_frag(struct jffs2_sb_info *c,
> >                         dbg_fragtree2("marking old node @0x%08x (0x%04x-0x%04x) obsolete\n",
> >                                 ref_offset(this->node->raw), this->node->ofs, this->node->ofs+this->node->size);
> >                         jffs2_mark_node_obsolete(c, this->node->raw);
> > +                       this->node->raw = NULL;
> >                         jffs2_free_full_dnode(this->node);
> >                 } else {
> 
> This is just noise. There's no point in setting foo->raw to NULL
> immediately before we free foo->raw anyway. Most of the rest of this
> patch is of this form, but there are one or two cases which aren't and
> perhaps merit closer investigation.
> 

Well, I just did what I belived you wanted me to do so I did not think
much about it. I can redo the first patch with the necessary part or you
can do it with any extra checks you deem necessary. I just want some
variant in mtd git so I can have the same to avoid conflicts later on.

  Jocke