Re: [PATCH 3/3] MIPS h/w watchpoint in GDBserver

[Pedro Alves <palves@redhat.com>]

On 06/13/2013 05:11 AM, Yao Qi wrote:
> On 05/30/2013 10:44 AM, Yao Qi wrote:
>> @@ -20,6 +20,7 @@
>>   #include "linux-low.h"
>>   
>>   #include <sys/ptrace.h>
>> +#include "mips-linux-watch.h"
>>   #include <endian.h>
>>   
>>   #include "gdb_proc_service.h"
> 
> Pedro mentioned that there are "unexplained odd placements for
> includes", however, the only 'oddity' I can see is the order of include
> mips-linux-watch.h and endian.h.  In this version, I exchange the order
> of them.

Thanks.  Yes, that's the oddity I was referring to, specifically:

 #include <system header>
 #include "local header"
 #include <system header>
 #include "local header"

IMO, it's better to keep system header includes all together, and
local includes all together (though we're not that good at doing that).
With that in mind, given the odd placement for "mips-linux-watch.h" between
two system headers, I wondered whether there was something
in "mips-linux-watch.h" or <endian.h> that made that particular placement
necessary, and if so, that should be explained, hence my "unexplained".

(IMO2, I'd go as far as saying that it's better to put system includes
after local includes, in order to prevent hidden dependencies in our
headers, following the principle that headers should be self contained,
but pull in the least dependencies possible with forward declarations
for opaque types, etc., and compileable on their own.)

-- 
Pedro Alves