Re: [dev] dmenu_path rewrite in C

On Wed, 19 May 2010 16:11:15 +0300
Elmo Todurov <todurov_AT_gmail.com> wrote:

> > 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.

and filesystem. in reiser there's some lag on first acceses.

> > 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.
i dont see the point of having:
char *moo(){
 char *foo;
 foo = strdup("bar");
 return foo;
}

when you can have:

char *moo() {
 return strdup ("bar");
}

> > 7)
> > I would change --force flag check to be just '-f'
>
> A matter of taste.

in suckless, getopt_long should not be used.

> > 8)
> > why do you check for root?
>
> Why do you use dmenu_path as root? I can't see any use cases for this.

I never use this as root, but why put a limitation, and have those bloated 5 LOCs? :)

> > 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).

it's not that 'complex'. code shouldnt look uglier with this change, it's just to
replace current allocator, which you should do, because failed mallocs must die().

Another optimization you can do is to remove the 'free()' part. It's useless and
slowdowns the execution. The kernel already frees the heap of the process on
exit(), and it's not a program that will run forever. Only until the end of the
execution.

This change will also speed up the execution.

> > 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.
>

no, I mean relative one from home. But this is just up to discussion, I would probably
never change this, and probably most of the people will not do it, so it's ok as is.

--pancake
Received on Wed May 19 2010 - 14:15:30 UTC