freemyipod r433 - Code Review

Jump to: navigation, search
Repository:freemyipod
Revision:r432‎ | r433 | r434 >
Date:16:53, 16 January 2011
Author:theseven
Status:new
Tags:
Comment:
emCORE: Fix some issues and make files/dirs runtime-allocated
Modified paths:
  • /emcore/trunk/button.c (modified) (history)
  • /emcore/trunk/dir.c (modified) (history)
  • /emcore/trunk/dir.h (modified) (history)
  • /emcore/trunk/file.c (modified) (history)
  • /emcore/trunk/file.h (modified) (history)

Diff [purge]

Index: emcore/trunk/button.c
@@ -53,22 +53,25 @@
5454 {
5555 struct button_hook_entry* h;
5656 struct button_hook_entry* handle = NULL;
57 - int result = 0;
 57+ int result = -1;
5858 mutex_lock(&button_mutex, TIMEOUT_BLOCK);
5959 if (head_button_hook && head_button_hook->handler == handler)
6060 {
6161 handle = head_button_hook;
6262 head_button_hook = head_button_hook->next;
 63+ free(handle);
 64+ result = 0;
6365 }
6466 else
6567 {
66 - for (h = head_button_hook; h && h->next->handler != handler; h = h->next);
67 - if (h)
68 - {
69 - handle = h->next;
70 - h->next = h->next->next;
71 - }
72 - else result = -1;
 68+ for (h = head_button_hook; h && h->next; h = h->next);
 69+ if (h->next->handler != handler)
 70+ {
 71+ handle = h->next;
 72+ h->next = h->next->next;
 73+ free(handle);
 74+ result = 0;
 75+ }
7376 }
7477 mutex_unlock(&button_mutex);
7578 if (handle) free(handle);
@@ -96,7 +99,7 @@
97100 head_button_hook = head_button_hook->next;
98101 free(prev);
99102 }
100 - for (h = head_button_hook->next; h; h = h->next)
 103+ for (h = head_button_hook; h; h = h->next)
101104 {
102105 while (h && h->owner == process)
103106 {
Index: emcore/trunk/dir.c
@@ -26,31 +26,46 @@
2727 #include "debug.h"
2828 #include "thread.h"
2929
30 -#ifndef MAX_OPEN_DIRS
31 -#define MAX_OPEN_DIRS 16
32 -#endif
 30+static struct mutex dir_mutex;
 31+static DIR* opendirs;
3332
34 -static DIR opendirs[MAX_OPEN_DIRS];
35 -
3633 #ifdef HAVE_HOTSWAP
3734 // release all dir handles on a given volume "by force", to avoid leaks
3835 int release_dirs(int volume)
3936 {
40 - DIR* pdir = opendirs;
41 - int dd;
 37+ DIR* prev;
4238 int closed = 0;
43 - for ( dd=0; dd<MAX_OPEN_DIRS; dd++, pdir++)
 39+ mutex_lock(&dir_mutex, TIMEOUT_BLOCK);
 40+#ifdef HAVE_MULTIVOLUME
 41+ DIR* d;
 42+ while (opendirs && opendirs->fatdir.file.volume == volume)
4443 {
45 -#ifdef HAVE_MULTIVOLUME
46 - if (pdir->fatdir.file.volume == volume)
47 -#else
48 - (void)volume;
49 -#endif
 44+ prev = opendirs;
 45+ opendirs = opendirs->next;
 46+ free(prev);
 47+ closed++;
 48+ }
 49+ for (d = opendirs; d; d = d->next)
 50+ {
 51+ while (d && d->fatdir.file.volume == volume)
5052 {
51 - pdir->busy = false; /* mark as available, no further action */
 53+ prev->next = d->next;
 54+ free(d);
5255 closed++;
5356 }
 57+ prev = d;
5458 }
 59+#else
 60+ (void)volume;
 61+ while (opendirs)
 62+ {
 63+ prev = opendirs;
 64+ opendirs = opendirs->next;
 65+ free(prev);
 66+ closed++;
 67+ }
 68+#endif
 69+ mutex_unlock(&dir_mutex);
5570 return closed; /* return how many we did */
5671 }
5772 #endif /* #ifdef HAVE_HOTSWAP */
@@ -61,8 +76,7 @@
6277 char* part;
6378 char* end;
6479 struct fat_direntry entry;
65 - int dd;
66 - DIR* pdir = opendirs;
 80+ DIR* pdir;
6781 #ifdef HAVE_MULTIVOLUME
6882 int volume;
6983 #endif
@@ -72,18 +86,8 @@
7387 return NULL;
7488 }
7589
76 - /* find a free dir descriptor */
77 - for ( dd=0; dd<MAX_OPEN_DIRS; dd++, pdir++)
78 - if ( !pdir->busy )
79 - break;
80 -
81 - if ( dd == MAX_OPEN_DIRS ) {
82 - DEBUGF("Too many dirs open");
83 - errno = EMFILE;
84 - return NULL;
85 - }
86 -
87 - pdir->busy = true;
 90+ pdir = (DIR*)memalign(0x10, sizeof(DIR));
 91+ if (!pdir) return NULL;
8892 pdir->process = current_thread;
8993
9094 #ifdef HAVE_MULTIVOLUME
@@ -96,7 +100,7 @@
97101
98102 if ( fat_opendir(IF_MV2(volume,) &pdir->fatdir, 0, NULL) < 0 ) {
99103 DEBUGF("Failed opening root dir");
100 - pdir->busy = false;
 104+ free(pdir);
101105 return NULL;
102106 }
103107
@@ -106,7 +110,7 @@
107111 while (1) {
108112 if ((fat_getnext(&pdir->fatdir,&entry) < 0) ||
109113 (!entry.name[0])) {
110 - pdir->busy = false;
 114+ free(pdir);
111115 return NULL;
112116 }
113117 if ( (entry.attr & FAT_ATTR_DIRECTORY) &&
@@ -127,7 +131,7 @@
128132 &pdir->fatdir) < 0 ) {
129133 DEBUGF("Failed opening dir '%s' (%ld)",
130134 part, entry.firstcluster);
131 - pdir->busy = false;
 135+ free(pdir);
132136 return NULL;
133137 }
134138 #ifdef HAVE_MULTIVOLUME
@@ -138,28 +142,53 @@
139143 }
140144 }
141145
 146+ mutex_lock(&dir_mutex, TIMEOUT_BLOCK);
 147+ pdir->next = opendirs;
 148+ opendirs = pdir;
 149+ mutex_unlock(&dir_mutex);
 150+
142151 return pdir;
143152 }
144153
145154 int closedir(DIR* dir)
146155 {
147 - dir->busy=false;
 156+ if (!dir) return 0;
 157+ DIR* d;
 158+ mutex_lock(&dir_mutex, TIMEOUT_BLOCK);
 159+ if (opendirs == dir) opendirs = dir->next;
 160+ else
 161+ for (d = opendirs; d; d = d->next)
 162+ if (d->next == dir)
 163+ d->next = dir->next;
 164+ mutex_unlock(&dir_mutex);
 165+ free(dir);
148166 return 0;
149167 }
150168
151169 int closedir_all_of_process(struct scheduler_thread* process)
152170 {
153 - DIR* pdir = opendirs;
154 - int dd;
 171+ DIR* d;
 172+ DIR* prev;
155173 int closed = 0;
156 - for ( dd=0; dd<MAX_OPEN_DIRS; dd++, pdir++)
 174+ mutex_lock(&dir_mutex, TIMEOUT_BLOCK);
 175+ while (opendirs && opendirs->process == process)
157176 {
158 - if (pdir->process == process)
 177+ prev = opendirs;
 178+ opendirs = opendirs->next;
 179+ free(prev);
 180+ closed++;
 181+ }
 182+ for (d = opendirs; d; d = d->next)
 183+ {
 184+ while (d && d->process == process)
159185 {
160 - pdir->busy = false; /* mark as available, no further action */
 186+ prev->next = d->next;
 187+ free(d);
161188 closed++;
162189 }
 190+ prev = d;
163191 }
 192+ mutex_unlock(&dir_mutex);
164193 return closed; /* return how many we did */
165194 }
166195
@@ -168,8 +197,7 @@
169198 struct fat_direntry entry;
170199 struct dirent* theent = &(dir->theent);
171200
172 - if (!dir->busy)
173 - return NULL;
 201+ if (!dir) return NULL;
174202
175203 #ifdef HAVE_MULTIVOLUME
176204 /* Volumes (secondary file systems) get inserted into the root directory
Index: emcore/trunk/file.c
@@ -37,20 +37,21 @@
3838 */
3939
4040 struct filedesc {
41 - unsigned char cache[SECTOR_SIZE] CACHEALIGN_ATTR;
 41+ unsigned char cache[SECTOR_SIZE];
4242 int cacheoffset; /* invariant: 0 <= cacheoffset <= SECTOR_SIZE */
4343 long fileoffset;
4444 long size;
4545 int attr;
4646 struct fat_file fatfile;
47 - bool busy;
 47+ struct filedesc* next;
 48+ struct scheduler_thread* process;
4849 bool write;
4950 bool dirty;
5051 bool trunc;
51 - struct scheduler_thread* process;
52 -} CACHEALIGN_ATTR;
 52+};
5353
54 -static struct filedesc openfiles[MAX_OPEN_FILES] CACHEALIGN_ATTR;
 54+static struct mutex file_mutex;
 55+static struct filedesc* openfiles;
5556
5657 static int flush_cache(int fd);
5758
@@ -63,10 +64,9 @@
6465 {
6566 DIR* dir;
6667 struct dirent* entry;
67 - int fd;
6868 char pathnamecopy[MAX_PATH];
6969 char* name;
70 - struct filedesc* file = NULL;
 70+ struct filedesc* file;
7171 int rc;
7272
7373 DEBUGF("open(\"%s\",%d)",pathname,flags);
@@ -78,20 +78,14 @@
7979 return -1;
8080 }
8181
82 - /* find a free file descriptor */
83 - for ( fd=0; fd<MAX_OPEN_FILES; fd++ )
84 - if ( !openfiles[fd].busy )
85 - break;
86 -
87 - if ( fd == MAX_OPEN_FILES ) {
88 - DEBUGF("Too many files open");
 82+ file = (struct filedesc*)memalign(0x10, sizeof(struct filedesc));
 83+ if (!file)
 84+ {
8985 errno = EMFILE;
9086 return -2;
9187 }
92 -
93 - file = &openfiles[fd];
9488 memset(file, 0, sizeof(struct filedesc));
95 -
 89+ file->process = current_thread;
9690 if (flags & (O_RDWR | O_WRONLY)) {
9791 file->write = true;
9892
@@ -98,8 +92,6 @@
9993 if (flags & O_TRUNC)
10094 file->trunc = true;
10195 }
102 - file->busy = true;
103 - file->process = current_thread;
10496
10597 strlcpy(pathnamecopy, pathname, sizeof(pathnamecopy));
10698
@@ -118,7 +110,7 @@
119111 if (!dir) {
120112 DEBUGF("Failed opening dir");
121113 errno = EIO;
122 - file->busy = false;
 114+ free(file);
123115 return -4;
124116 }
125117
@@ -125,7 +117,7 @@
126118 if(name[0] == 0) {
127119 DEBUGF("Empty file name");
128120 errno = EINVAL;
129 - file->busy = false;
 121+ free(file);
130122 closedir(dir);
131123 return -5;
132124 }
@@ -152,7 +144,7 @@
153145 if (rc < 0) {
154146 DEBUGF("Couldn't create %s in %s",name,pathnamecopy);
155147 errno = EIO;
156 - file->busy = false;
 148+ free(file);
157149 closedir(dir);
158150 return rc * 10 - 6;
159151 }
@@ -162,7 +154,7 @@
163155 else {
164156 DEBUGF("Couldn't find %s in %s",name,pathnamecopy);
165157 errno = ENOENT;
166 - file->busy = false;
 158+ free(file);
167159 closedir(dir);
168160 return -7;
169161 }
@@ -169,7 +161,7 @@
170162 } else {
171163 if(file->write && (file->attr & FAT_ATTR_DIRECTORY)) {
172164 errno = EISDIR;
173 - file->busy = false;
 165+ free(file);
174166 closedir(dir);
175167 return -8;
176168 }
@@ -180,12 +172,20 @@
181173 file->fileoffset = 0;
182174
183175 if (file->write && (flags & O_APPEND)) {
184 - rc = lseek(fd,0,SEEK_END);
 176+ rc = lseek((int)file,0,SEEK_END);
185177 if (rc < 0 )
 178+ {
 179+ free(file);
186180 return rc * 10 - 9;
 181+ }
187182 }
188183
189 - return fd;
 184+ mutex_lock(&file_mutex, TIMEOUT_BLOCK);
 185+ file->next = openfiles;
 186+ openfiles = file;
 187+ mutex_unlock(&file_mutex);
 188+
 189+ return (int)file;
190190 }
191191
192192 int file_open(const char* pathname, int flags)
@@ -196,57 +196,68 @@
197197
198198 int close(int fd)
199199 {
200 - struct filedesc* file = &openfiles[fd];
201200 int rc = 0;
202 -
 201+ struct filedesc* f = (struct filedesc*)fd;
203202 DEBUGF("close(%d)", fd);
204 -
205 - if (fd < 0 || fd > MAX_OPEN_FILES-1) {
206 - errno = EINVAL;
207 - return -1;
208 - }
209 - if (!file->busy) {
 203+ if (!f)
 204+ {
210205 errno = EBADF;
211206 return -2;
212207 }
213 - if (file->write) {
 208+ if (f->write)
 209+ {
214210 rc = fsync(fd);
215211 if (rc < 0)
216212 return rc * 10 - 3;
217213 }
218 -
219 - file->busy = false;
 214+ mutex_lock(&file_mutex, TIMEOUT_BLOCK);
 215+ if (openfiles == f) openfiles = openfiles->next;
 216+ else
 217+ for (f = openfiles; f; f = f->next)
 218+ if ((int)(f->next) == fd)
 219+ f->next = f->next->next;
 220+ mutex_unlock(&file_mutex);
 221+ free((void*)fd);
220222 return 0;
221223 }
222224
223225 int close_all_of_process(struct scheduler_thread* process)
224226 {
225 - struct filedesc* pfile = openfiles;
226 - int fd;
 227+ struct filedesc* f;
 228+ struct filedesc* prev;
227229 int closed = 0;
228 - for ( fd=0; fd<MAX_OPEN_FILES; fd++, pfile++)
 230+ mutex_lock(&file_mutex, TIMEOUT_BLOCK);
 231+ while (openfiles && openfiles->process == process)
229232 {
230 - if (pfile->process == process)
 233+ prev = openfiles;
 234+ openfiles = openfiles->next;
 235+ if (openfiles->write) fsync((int)openfiles);
 236+ free(prev);
 237+ closed++;
 238+ }
 239+ for (f = openfiles; f; f = f->next)
 240+ {
 241+ while (f && f->process == process)
231242 {
232 - pfile->busy = false; /* mark as available, no further action */
 243+ prev->next = f->next;
 244+ if (f->write) fsync((int)f);
 245+ free(f);
233246 closed++;
234247 }
 248+ prev = f;
235249 }
 250+ mutex_unlock(&file_mutex);
236251 return closed; /* return how many we did */
237252 }
238253
239254 int fsync(int fd)
240255 {
241 - struct filedesc* file = &openfiles[fd];
 256+ struct filedesc* file = (struct filedesc*)fd;
242257 int rc = 0;
243258
244259 DEBUGF("fsync(%d)", fd);
245260
246 - if (fd < 0 || fd > MAX_OPEN_FILES-1) {
247 - errno = EINVAL;
248 - return -1;
249 - }
250 - if (!file->busy) {
 261+ if (!file) {
251262 errno = EBADF;
252263 return -2;
253264 }
@@ -364,7 +375,7 @@
365376 return - 5;
366377 }
367378
368 - file = &openfiles[fd];
 379+ file = (struct filedesc*)fd;
369380
370381 rc = fat_rename(&file->fatfile, &dir->fatdir, nameptr,
371382 file->size, file->attr);
@@ -404,7 +415,7 @@
405416 int ftruncate(int fd, off_t size)
406417 {
407418 int rc, sector;
408 - struct filedesc* file = &openfiles[fd];
 419+ struct filedesc* file = (struct filedesc*)fd;
409420
410421 sector = size / SECTOR_SIZE;
411422 if (size % SECTOR_SIZE)
@@ -430,7 +441,7 @@
431442 static int flush_cache(int fd)
432443 {
433444 int rc;
434 - struct filedesc* file = &openfiles[fd];
 445+ struct filedesc* file = (struct filedesc*)fd;
435446 long sector = file->fileoffset / SECTOR_SIZE;
436447
437448 DEBUGF("Flushing dirty sector cache");
@@ -462,14 +473,9 @@
463474 struct filedesc* file;
464475 int rc, rc2;
465476
466 - if (fd < 0 || fd > MAX_OPEN_FILES-1) {
467 - errno = EINVAL;
468 - return -1;
469 - }
 477+ file = (struct filedesc*)fd;
470478
471 - file = &openfiles[fd];
472 -
473 - if ( !file->busy ) {
 479+ if (!file) {
474480 errno = EBADF;
475481 return -1;
476482 }
@@ -643,7 +649,7 @@
644650
645651 ssize_t write(int fd, const void* buf, size_t count)
646652 {
647 - if (!openfiles[fd].write) {
 653+ if (!((struct filedesc*)fd)->write) {
648654 errno = EACCES;
649655 return -1;
650656 }
@@ -663,15 +669,11 @@
664670 long oldsector;
665671 int sectoroffset;
666672 int rc;
667 - struct filedesc* file = &openfiles[fd];
 673+ struct filedesc* file = (struct filedesc*)fd;
668674
669675 DEBUGF("lseek(%d,%ld,%d)",fd,offset,whence);
670676
671 - if (fd < 0 || fd > MAX_OPEN_FILES-1) {
672 - errno = EINVAL;
673 - return -1;
674 - }
675 - if ( !file->busy ) {
 677+ if (!file) {
676678 errno = EBADF;
677679 return -1;
678680 }
@@ -741,13 +743,9 @@
742744
743745 off_t filesize(int fd)
744746 {
745 - struct filedesc* file = &openfiles[fd];
 747+ struct filedesc* file = (struct filedesc*)fd;
746748
747 - if (fd < 0 || fd > MAX_OPEN_FILES-1) {
748 - errno = EINVAL;
749 - return -1;
750 - }
751 - if ( !file->busy ) {
 749+ if (!file) {
752750 errno = EBADF;
753751 return -1;
754752 }
@@ -760,21 +758,39 @@
761759 /* release all file handles on a given volume "by force", to avoid leaks */
762760 int release_files(int volume)
763761 {
764 - struct filedesc* pfile = openfiles;
765 - int fd;
 762+ struct filedesc* prev;
766763 int closed = 0;
767 - for ( fd=0; fd<MAX_OPEN_FILES; fd++, pfile++)
 764+ mutex_lock(&file_mutex, TIMEOUT_BLOCK);
 765+#ifdef HAVE_MULTIVOLUME
 766+ struct filedesc* f;
 767+ while (openfiles && openfiles->fatfile.volume == volume)
768768 {
769 -#ifdef HAVE_MULTIVOLUME
770 - if (pfile->fatfile.volume == volume)
771 -#else
772 - (void)volume;
773 -#endif
 769+ prev = openfiles;
 770+ openfiles = openfiles->next;
 771+ free(prev);
 772+ closed++;
 773+ }
 774+ for (f = openfiles; f; f = f->next)
 775+ {
 776+ while (f && f->fatfile.volume == volume)
774777 {
775 - pfile->busy = false; /* mark as available, no further action */
 778+ prev->next = f->next;
 779+ free(f);
776780 closed++;
777781 }
 782+ prev = f;
778783 }
 784+#else
 785+ (void)volume;
 786+ while (openfiles)
 787+ {
 788+ prev = openfiles;
 789+ openfiles = openfiles->next;
 790+ free(prev);
 791+ closed++;
 792+ }
 793+#endif
 794+ mutex_unlock(&file_mutex);
779795 return closed; /* return how many we did */
780796 }
781797 #endif /* #ifdef HAVE_HOTSWAP */
Index: emcore/trunk/dir.h
@@ -47,8 +47,8 @@
4848 #else
4949 #include "fat.h"
5050
51 -typedef struct {
52 - bool busy;
 51+typedef struct dirdesc {
 52+ struct dirdesc* next;
5353 long startcluster;
5454 struct fat_dir fatdir;
5555 struct dirent theent;
Index: emcore/trunk/file.h
@@ -28,10 +28,6 @@
2929 #include "libc/include/_ansi.h"
3030 #include "thread.h"
3131
32 -#ifndef MAX_OPEN_FILES
33 -#define MAX_OPEN_FILES 32
34 -#endif
35 -
3632 #ifndef SEEK_SET
3733 #define SEEK_SET 0
3834 #endif