This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: bfd patch for gdb cross-debugging support


Danny Backx <danny.backx@scarlet.be> writes:

>> > +#ifndef FALSE
>> > +#define FALSE 0
>> > +#endif
>> > +#ifndef TRUE
>> > +#define TRUE 1
>> >  #endif
>> 
>> Please don't add unnecessary preprocessor macros.
>
> Ah, I see that these are defined in bfd/bfd-in2.h . Am I to include that
> inside filenames.h ?

No, just use 0 and 1 in the macro expansions.  Why do you need TRUE and
FALSE?


>> > +extern int have_dos_based_file_system;
>> 
>> If we're going to change this to require a .c file, I personally would
>> much prefer to see a function based interface rather than one based on a
>> global variable.
>
> So I can change this variable into a function that reads the contents of
> a static variable hidden in bfd/init.c or so ? Obviously then I'd also
> have to implement a function to set its value, in the same source.
>
> If I do this, is the consequence also that I replace all occurrences of
> HAVE_DOS_BASED_FILE_SYSTEM by uses of this new function, or should I go
> for a minimalistic change ?

Think bigger: instead of having a function which returns whether or not
you are on a DOS based file system, find the cases which test that and
write functions which implement the functionality that they need.  Have
those functions test your new global variable.  You already need to
change that code anyhow.

To be clear, I am not making requirements for this commit.  That is up
to Nick.  I'm saying what I think is the right thing to do here for
long-term maintainability and ease of use.

Aside from API issues, I will note that using a global variable is not
an ideal choice when BFD is compiled as a shared library.

Ian


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]