|
| 1 | +A laundry list of things that need looking at, most of which will |
| 2 | +require more work than the average checkpatch cleanup... |
| 3 | + |
| 4 | +Note that some of these entries may not be bugs - they're things |
| 5 | +that need to be looked at, and *possibly* fixed. |
| 6 | + |
| 7 | +Clean up the ffsCamelCase function names. |
| 8 | + |
| 9 | +Fix (thing)->flags to not use magic numbers - multiple offenders |
| 10 | + |
| 11 | +Sort out all the s32/u32/u8 nonsense - most of these should be plain int. |
| 12 | + |
1 | 13 | exfat_core.c - ffsReadFile - the goto err_out seem to leak a brelse(). |
2 | 14 | same for ffsWriteFile. |
3 | 15 |
|
4 | | -exfat_core.c - fs_sync(sb,0) all over the place looks fishy as hell. |
5 | | -There's only one place that calls it with a non-zero argument. |
| 16 | +All the calls to fs_sync() need to be looked at, particularly in the |
| 17 | +context of EXFAT_DELAYED_SYNC. Currently, if that's defined, we only |
| 18 | +flush to disk when sync() gets called. We should be doing at least |
| 19 | +metadata flushes at appropriate times. |
6 | 20 |
|
7 | 21 | ffsTruncateFile - if (old_size <= new_size) { |
8 | 22 | That doesn't look right. How did it ever work? Are they relying on lazy |
9 | 23 | block allocation when actual writes happen? If nothing else, it never |
10 | 24 | does the 'fid->size = new_size' and do the inode update.... |
11 | 25 |
|
12 | 26 | ffsSetAttr() is just dangling in the breeze, not wired up at all... |
| 27 | + |
| 28 | +Convert global mutexes to a per-superblock mutex. |
| 29 | + |
| 30 | +Right now, we load exactly one UTF-8 table. Check to see |
| 31 | +if that plays nice with different codepage and iocharset values |
| 32 | +for simultanous mounts of different devices |
| 33 | + |
| 34 | +exfat_rmdir() checks for -EBUSY but ffsRemoveDir() doesn't return it. |
| 35 | +In fact, there's a complete lack of -EBUSY testing anywhere. |
| 36 | + |
| 37 | +There's probably a few missing checks for -EEXIST |
| 38 | + |
| 39 | +check return codes of sync_dirty_buffer() |
| 40 | + |
| 41 | +Why is remove_file doing a num_entries++?? |
| 42 | + |
| 43 | +Double check a lot of can't-happen parameter checks (for null pointers for |
| 44 | +things that have only one call site and can't pass a null, etc). |
| 45 | + |
| 46 | +All the DEBUG stuff can probably be tossed, including the ioctl(). Either |
| 47 | +that, or convert to a proper fault-injection system. |
| 48 | + |
| 49 | +exfat_remount does exactly one thing. Fix to actually deal with remount |
| 50 | +options, particularly handling R/O correctly. For that matter, allow |
| 51 | +R/O mounts in the first place. |
| 52 | + |
| 53 | +Figure out why the VFAT code used multi_sector_(read|write) but the |
| 54 | +exfat code doesn't use it. The difference matters on SSDs with wear leveling. |
| 55 | + |
| 56 | +exfat_fat_sync(), exfat_buf_sync(), and sync_alloc_bitmap() |
| 57 | +aren't called anyplace.... |
| 58 | + |
| 59 | +Create helper function for exfat_set_entry_time() and exfat_set_entry_type() |
| 60 | +because it's sort of ugly to be calling the same functionn directly and |
| 61 | +other code calling through the fs_func struc ponters... |
| 62 | + |
| 63 | +clean up the remaining vol_type checks, which are of two types: |
| 64 | +some are ?: operators with magic numbers, and the rest are places |
| 65 | +where we're doing stuff with '.' and '..'. |
| 66 | + |
| 67 | +Patches to: |
| 68 | + Greg Kroah-Hartman < [email protected]> |
| 69 | + Valdis Kletnieks < [email protected]> |
0 commit comments