2020-05-27 17:00 GMT+09:00, Kohada.Tetsuhiro@xxxxxxxxxxxxxxxxxxxxxxxxxxx <Kohada.Tetsuhiro@xxxxxxxxxxxxxxxxxxxxxxxxxxx>: > Thank you for your comment. > > >> + for (i = 0; i < es->num_bh; i++) { > >> + if (es->modified) > >> + exfat_update_bh(es->sb, es->bh[i], sync); > > > > Overall, it looks good to me. > > However, if "sync" is set, it looks better to return the result of > exfat_update_bh(). > > Of course, a tiny modification for exfat_update_bh() is also required. > > I thought the same, while creating this patch. > However this patch has changed a lot and I didn't add any new error > checking. > (So, the same behavior will occur even if an error occurs) > > >> +struct exfat_dentry *exfat_get_dentry_cached( > >> + struct exfat_entry_set_cache *es, int num) { > >> + int off = es->start_off + num * DENTRY_SIZE; > >> + struct buffer_head *bh = es->bh[EXFAT_B_TO_BLK(off, es->sb)]; > >> + char *p = bh->b_data + EXFAT_BLK_OFFSET(off, es->sb); > > > > In order to prevent illegal accesses to bh and dentries, it would be > better to check validation for num and bh. > > There is no new error checking for same reason as above. > > I'll try to add error checking to this v2 patch. > Or is it better to add error checking in another patch? The latter:) Thanks! > > BR > --- > Kohada Tetsuhiro <Kohada.Tetsuhiro@xxxxxxxxxxxxxxxxxxxxxxxxxxx>