ratpoison-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [RP] select patch


From: Jeff Abrahamson
Subject: Re: [RP] select patch
Date: Thu, 19 Jun 2014 18:25:23 +0100

Hi, Johannes.

Thanks, this is very nice.  Some more comments and suggestions.

Why do you need MATCH_PREFIX_IGNORECASE instead of just using MATCH_PREFIX | MATCH_IGNORECASE ?

Is MAX_WINDOW_NAME_LENGTH == 42 reasonable?  The goal isn't to enforce short window names, only to protect against strings not being null-terminated.  Why not 1024 or even 4096?  Or maybe something already enforces 42?

I'm a fan of descriptive variable names.  So compare_length is more descriptive to me than cmplen.  This is motivated by seeing abbreviations that were obvious after being explained but that required asking.

Many a bug can be prevented by using braces even on one line blocks where they are not needed.  The mechanism is that someone adds a statement later on and misses the missing braces.

window_name returns a char* that is not a copy (i.e., you don't own it).  So modifying it isn't such a good idea.

Can you use a function pointer for your compare function so that you don't need to have the pre-processing block?


Jeff Abrahamson
+33 6 24 40 01 57   <-- brièvement indisponible le 4 juillet
+44 7920 594 255    <-- will change 18 July

http://jeff.purple.com/
http://blog.purple.com/jeff/



On 19 June 2014 18:16, Johannes Altmanninger <address@hidden> wrote:

On 06/19/2014 07:07 PM, Johannes Altmanninger wrote:

#define MATCH_PREFIX_IGNORECASE 0x0004
my bad this should be
#define MATCH_PREFIX_IGNORECASE 0x0003


reply via email to

[Prev in Thread] Current Thread [Next in Thread]