On 05/19/2010 02:24 PM, pancake wrote:
1) In unix there's a MAXPATH variable.. in fact GNU does not have this limit, but in unix is 4096 and in plan9 256 (afaik)
Actually, PATH_MAX.
the thing is that keeping a clean system you shouldn't have paths that big.
agreed
So you can define a single buffer of this size to strcpy/memcpy/strcat the paths you need to construct the executable paths you need. this will reduce the heap usage a lot.
This approach would also add complexity. I would guess that disk IO is the limiting factor to the speed, not malloc.
the memory accesses will be hardly reduced because you can keep the basedir on the buffer on all iterations for each directory. only changing filename.
Hmm, this is a good point. I think I'll implement (at least some of) it.
2) syntax is not following the suckless style, I would prefer to have all source files in suckless following the same rules.
That's easy to fix, and arguably unnecessary.
3) There'r many places where you don't check for result of malloc/getenv is null.
Now I do. Haven't finished the cleanup yet, though.
4) many variables can be removed (like copy_path in get_PATH()
The question is not whether they can be removed. SHOULD they be removed, this is the question for me.
5) I would add 'die()' instead of perror+exit
Done
6) use sizeof(buf) instead of hardcoded size (makes the code safer to changes)
Fixed
7) I would change --force flag check to be just '-f'
A matter of taste.
8) why do you check for root?
Why do you use dmenu_path as root? I can't see any use cases for this.
9) as all filepaths are of similar size you can just allocate blocks of this size and index by using a multiplier which results faster than having many chunks (with some tests i did in 'alt' (hg.youterm.com/alt) this kind of optimizations result in 3-5x faster execution.
And would also add much confusion to the reader (and, judging by the mailing list, our readers seem to have sensitive eyes).
10) put ".dmenu_cache" path as define on top of C file. so you can change it easily.
Full path? No thanks, it better sit at $HOME.
