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

PenguinPlay.h analysis



[Caution - long mail]

Ok, I couldn't hold me back anymore <grin> - here's my comments about
PenguinPlay.h (the stuff in in lines beginning with '>'):

----------- PenguinPlay.h ------------
/****************************************************
  PenguinPlay.h:

  This header defines types and utilitly functions the
  PenguinPlay Game Development Kit.

  This file (but not all of PenguinPlay) is in the public domain, so
  feel free to do whatever you like to it (except copyright it).
***/

#ifndef _PenguinPlay_h
#define _PenguinPlay_h

> What convention do we have for these macros? I used __PENGUINPLAY_H__ 
> up to now. Perhaps we should write some "PenguinPlay coding standards"
> doc ;)

#include <config.h>

#include <unistd.h>

> Stop! You can't be sure that unistd.h exists - that's why I inserted the
> buggy AC_CHECK_HEADERS(unistd.h) test in configure.in

#include <limits.h>
#include <stdio.h>

/***************************************
*
*  Examine our environment.
*
**************************************/

/***
  What's that bit of silicon under us?
***/

#ifndef _PP_ARCH_GENERIC /*For unknown architectures or testing*/

#if defined(__386__) || defined(i386)
  #define _PP_ARCH_I386
#endif /*386 Architecture*/

> We should make a list of all constants defined by PenguinPlay
> (PenguinPlay.h, config.h, ...)

#endif /*!_PP_ARCH_GENERIC*/

/***
  What language are we using?
***/
#if defined(__cplusplus) || defined(c_plusplus)
  #define _PP_CPLUSPLUS
#else
#define _PP_C
#endif


/***
  Do 64 bit integers exist?
  Thanks Yong Chi for the hack for detecting
  64 bit architectures.
***/

#if INT_MAX < LONG_MAX

> sure that this is true for all 64bit archs?

  #define _PP_64BIT_ARCH
  #define _PP_64BIT_INTS
#elif defined (__GNUC__)
  #define _PP_64BIT_INTS

> IIRC the gcc docs state that long long can't be relied on for all things,
> e.g. multiplicating long longs isn't necessarily possible, taking their
> addresses is propably undefined etc. IMHO we shouldn't use them
> (or at least not define ppInt64 etc to be long long)

#endif


/*[en|disable] some GCC specifics*/
#ifdef __GNUC__
#else /*!GNUC*/
  #define __attribute__(ARG)
#endif /*!GNUC*/



/*
  Handle alloca: this probably needs fixing when we port.
*/
#ifdef HAVE_ALLOCA
  #define _PP_ALLOCA alloca
  #define _PP_DEALLOCA(PTR)
#else /*!HAVE_ALLOCA*/
  #define _PP_ALLOCA malloc
  #define _PP_DEALLOCA(PTR) free(PTR)
#endif


/***************************************
*
*  Miscilaneus definitions
*
**************************************/

#ifdef _PP_CPLUSPLUS
  #define _PP_EXTC extern "C"
#else
  #define _PP_EXTC extern

> that one should be empty instead of "extern", at least if it will be used
> for exten "C" {} blocks

#endif




/***************************************
*
*  Types and constants
*
**************************************/


/***
  Old favourites.
***/

/*
  Always felt that defining all these types is a bit excessive.
  But it seems like standard practice, so I'll do it anyway.
  (Besides, these are about the only original fragment of the
  sources I inherited from Steve Baker, so I'll keep them for
  nostalgic reasons:) )
*/
typedef          bool     ppBool   ;

> useful, but we can't rely on an existing "bool" type (or does autoconf
> define it ?)

typedef          int      ppInt    ;

> useless

typedef signed   char     ppByte   ;

> useful, but IMHO a Byte type should be unsigned

typedef          short    ppShort  ;
typedef unsigned int      ppuInt   ;

> Hmm, do we want to have shortcuts for unsigned types? 

typedef unsigned char     ppuByte  ;
typedef unsigned short    ppuShort ;
typedef          float    ppFloat  ;
typedef          double   ppDouble ;


/***
  Integers with guaranteed _minimum_ size.
  These could be larger than you expect,
  they are designed for speed.
***/
typedef          long int      ppInt32;
typedef          long int      ppInt16;
typedef          long int      ppInt8;
typedef unsigned long int      ppuInt32;
typedef unsigned long int      ppuInt16;
typedef unsigned long int      ppuInt8;

/*64 bits ints might exist.  Preferably becuase this is a 64
  bit architecture, but we will use the GCC long long type if we
  have to.
*/
#ifdef _PP_64BIT_INTS
#ifdef _PP_64BIT_ARCH
  typedef              long ppInt64;
  typedef  unsigned    long ppuInt64;
#elif defined(__GNUC__)
  typedef           long long ppInt64;
  typedef  unsigned long long ppuInt64;

> again: this could backfire

#endif
#endif


/***
  Integers with guaranteed _exact_ size.
***/
typedef          int      pp32;
typedef          short    pp16;
typedef signed   char     pp8;
typedef unsigned int      ppu32;
typedef unsigned short    ppu16;
typedef unsigned char     ppu8;
#ifdef _PP_64BIT_INTS
#ifdef _PP_64BIT_ARCH
  typedef              long pp64;
  typedef  unsigned    long ppu64;
#elif __GNUC__
  typedef          long long pp64;
  typedef unsigned long long ppu64;
#endif
#endif


/**
  This is used in the debugging versions of the messaging
  functions.
**/
#ifdef __GNUC__
  #define _PP_FUNCTION_NAME __PRETTY_FUNCTION__
#else
  extern const char* _ppNoFuncNameMsg;
  #define _PP_FUNCTION_NAME _ppNoFuncNameMsg;
#endif




/***************************************
*
*  Miscilaneus library functions.
*
**************************************/

/***
  Intializes the library with aid of command line.

  Unused options are returned in argv.  No stupid rules saying that argc
  and argv must be the real ones, make them up if you like.
***/
_PP_EXTC int ppInit (int* argc, char** argv) ;

> The code for this is in src/ppUtils.cc, right?

/***
  I think we should just let the user use exit()
***/
_PP_EXTC void ppShutdown () ;



/***************************************
*
*  Debugging stuff.
*
**************************************/

> see my mail concerning debug levels.

/*Debugging messages only if debugging is compiled in*/
#ifdef PP_DEBUG
  /*_ppSetLineNum(__LINE), _ppSet*/
  _PP_EXTC void _ppDbgDebug(const char* msg, ...)
    __attribute__((__format__(printf, 1, 2)));
  _PP_EXTC void _ppDbgWarning(const char* msg, ...)
    __attribute__((__format__(printf, 1, 2)));
  _PP_EXTC void _ppDbgFatalError(const char* msg, ...)
    __attribute__((__format__(printf, 1, 2)));

  /*like ppDebug but without line info*/
  _PP_EXTC void ppDbgPrint(const char* msg, ...)
    __attribute__((__format__(printf, 1, 2)));

  _PP_EXTC int* _pp_db_lineno_loc(void);
  _PP_EXTC char** _pp_db_file_loc(void);
  _PP_EXTC char** _pp_db_func_loc(void);

  #define ppDebug (\
      *_pp_db_lineno_loc()=__LINE__,\
      *_pp_db_file_loc()=__FILE__,\
      *_pp_db_func_loc()=_PP_FUNCTION_NAME,\
       _ppDbgDebug\
  )
  #define ppWarning (\
      *_pp_db_lineno_loc()=__LINE__,\
      *_pp_db_file_loc()=__FILE__,\
      *_pp_db_func_loc()=_PP_FUNCTION_NAME,\
       _ppDbgWarning\
  )
  #define ppFatalError (\
       *_pp_db_lineno_loc()=__LINE__,\
       *_pp_db_file_loc()=__FILE__,\
       *_pp_db_func_loc()=_PP_FUNCTION_NAME,\
        _ppDbgFatalError\
  )

#else /*PP_DEBUG*/
  _PP_EXTC void _ppWarning(const char* msg, ...)
    __attribute__((__format__(printf, 1, 2)));
  _PP_EXTC void _ppFatalError(const char* msg, ...)
    __attribute__((__format__(printf, 1, 2)));

  #define ppDebug (void)
  #define ppWarning _ppWarning
  #define ppFatalError _ppFatalError
  #define ppDbgPrint (void)
#endif /*!PP_DEBUG*/







#ifdef _PP_CPLUSPLUS


/***************************************
*
*  From here on in, we have  C++ specific stuff.
*  (FIX: Maybe this should be moved to a seperate file?).
*
**************************************/

/*
    Utility Definitions.
*/

/*************************
   I for one have hundreds of functions called GetXXX and SetXXX, which
   are just little inlines which access some data member of a class.

   So here are some macros which expand out to be common method declarations.
   I don't think these macros really harm readability.  In fact I think
   they improve it.  We have five types of GETSETS:

> But you won't beat me if I don't use them, ok? ;)

     _PP_GETSET:           Accesses a variable in a straigforward way.
	 _PP_GETSET_REF:       Accesses a reference.  Like Java.
	 _PP_GETSET_LINK:      Accesses a link counted object. (ppLinkCounted)
	 _PP_GETSET_VIRTUAL:   Declare GetXXX and SetXXX to be virtual
	 _PP_GETSET_ABSTRACT:  Declare GetXXX and SetXXX to be abstract

   All these are to be used in class definitions.  The first three expand
   to inline functions.

   Also there is:

   _PP_DEFINE_GETSET
   _PP_DEFINE_GETSET_REF
   _PP_DEFINE_GETSET_LINK

   Which can be used in .cc files to create out of line definitions.
   Useful but not required in conjunction with _PP_GETSET_VIRTUAL.

   FIX: We are losing a lot of type safety here.  What if we were to try
   using const_cast things instead of brute force typecasts?
***********************/

/* The following three macros are backends which do the actual work
 * for the _PP_GETSET* macros.
 */
#define _PP__GETSET(NAME, VAR, TYPE, CLASS)\
    TYPE CLASS##Get##NAME ()const        {return (TYPE)VAR;}\
    void CLASS##Set##NAME (const TYPE n) {VAR=(TYPE)n;}

/*as above, but does assignment by reference, rather than value.*/
#define _PP__GETSET_REF(NAME, VAR, TYPE, CLASS)\
    TYPE& CLASS##Get##NAME () const        {return *(TYPE*)VAR;}\
    void  CLASS##Set##NAME (const TYPE& n) {VAR=(TYPE*)&n;}

#define _PP__GETSET_LINK(NAME, VAR, TYPE, CLASS)\
    TYPE& CLASS##Get##NAME ()const         {return *(TYPE*)VAR;}\
    void  CLASS##Set##NAME (TYPE& n) {\
      if(VAR) VAR->Unlink();\
      if( (VAR=&n) ) VAR->Link();\
    }


#define _PP_GETSET(NAME, VAR, TYPE) _PP__GETSET(NAME, VAR, TYPE,)
#define _PP_GETSET_REF(NAME, VAR, TYPE) _PP__GETSET_REF(NAME, VAR, TYPE,)
#define _PP_GETSET_LINK(NAME, VAR, TYPE) _PP__GETSET_LINK(NAME, VAR, TYPE,)



#define _PP__V_GETSET(NAME, TYPE, ABST)\
    virtual TYPE Get##NAME()const ABST ;\
    virtual void Set##NAME(const TYPE n) ABST ;

#define _PP__V_GETSET_REF(NAME, TYPE, ABST)\
    virtual TYPE& Get##NAME()const ABST;\
    virtual void Set##NAME(const TYPE& n) ABST;

#define _PP__V_GETSET_LINK(NAME, TYPE, ABST)\
    virtual TYPE& Get##NAME()const ABST;\
    virtual void Set##NAME(TYPE &n) ABST;


#define _PP_VIRTUAL_GETSET(NAME, TYPE) _PP__V_GETSET(NAME,TYPE,)
#define _PP_VIRTUAL_GETSET_REF(NAME, TYPE) _PP__V_GETSET_REF(NAME,TYPE,)
#define _PP_VIRTUAL_GETSET_LINK(NAME, TYPE) _PP__V_GETSET_LINK(NAME,TYPE,)


#define _PP_ABSTRACT_GETSET(NAME, TYPE) _PP__V_GETSET(NAME, TYPE, =0)
#define _PP_ABSTRACT_GETSET_REF(NAME, TYPE) _PP__V_GETSET_REF(NAME, TYPE, =0)
#define _PP_ABSTRACT_GETSET_LINK(NAME, TYPE) _PP__V_GETSET_LINK(NAME, TYPE, =0)


#define _PP_DEFINE_GETSET(CLASS, NAME, VAR, TYPE)\
	_PP__GETSET(NAME, VAR, TYPE, CLASS##::)

#define _PP_DEFINE_GETSET_REF(CLASS, NAME, VAR, TYPE)\
	_PP__GETSET_REF(NAME, VAR, TYPE, CLASS##::)

#define _PP_DEFINE_GETSET_LINK(CLASS, NAME, VAR, TYPE)\
	_PP__GETSET_LINK(NAME, VAR, TYPE, CLASS##::)



/***
*  ppBase: Base class for everything.  Should do nothing when debugging
*  is not on.
***/

#ifdef PP_DEBUG

/*******************************************
  When debugging is on, we maintain a list of all objects derived from
  ppBase.  We can then get them to
       * give us a string the object.
       * dump their state info to a file.
       * test whether the current state is valid or garbage.
  All this is done by virtual functions with resonable defaults,
  so if you don't want to be bothered with this stuff in your
  own classes, you don't have to be.

  FIX: I don't know what happens when you use multiple inheritance.

> Perhaps all classes should use public virtual inheritance from ppBase?

*******************************************/
class ppBase {

  ppBase *next, *prev;
  static ppBase *first_object=NULL;
  static next_id;
  int id;

  public:


  /*
   * Stupid funcions for dumping deubging info.
   */

  virtual char* dbGetClassName()const{return "OBJECT";}
  virtual char* dbGetObjectName(char* buf, size_t len)const;
  char* dbLeekyGetObjectName()const
  {
    /*
      Please note, when not debugging, don't use functions
      which knowingly leek memory.

> My Oxford dictionary says:
> leek (n) - vegetable related to the onion but with wider green leaves
> above a long white bulb ;)
> Things get tasty here ;)

      ofcourse you could just delete the string aftwerwards.
	*/
    return dbGetObjectName(new char[100], 100);
  };

  //this is the only sane one to actuall use.  The two things above
  //it are just cruft. Note: it leeks. (That's the problem with char*).
  static char* dbLeekyGetObjectName(const ppBase& obj){
    static char* Null = "{NULL}";
    if(&obj)return obj.dbLeekyGetObjectName();
	else return strcpy(new char[sizeof(Null)], Null);
  }

  ppBase();

  /*The data in here must not be copied.*/
  ppBase(const ppBase& o){ ppBase(); }
  ppBase& operator = (const ppBase& o){return *this;}

  virtual ~ppBase();


  /*
   * Actuall work
   */
  /*be sure to call the inherited version if you override either of these.*/
  virtual bool dbIntegrityOk()const;
  virtual void dbDump(FILE* out)const;

  /*this does what it names suggest.*/
  static void dbDumpAll(FILE* out);

};

#define _PP_DB_STD_OBJNAME(NAME)

#else

class ppBase {

  public:

  ppBase(){}
  /*DON'T use virtual destructor, that incurrs overhead in unexcpected ways.*/
};


#define _PP_DB_STD_OBJNAME(NAME)

#endif /*PP_DEBUG*/






/****************************
Derive from this class if you wan't an object to have
its allocation handled by link counts.
****************************/
class ppLinkCounted : public ppBase {
  int link_count;

  public:

 /*
  *  Administrivia
  */

  ppLinkCounted():link_count(0){}

  ppLinkCounted(const ppLinkCounted& o)
  {
    link_count = 0;
  };

  ppLinkCounted& operator=(const ppLinkCounted& o)
  {
    /*this must be a no op to stop the link count from being changed.*/
	return *this;
  }

  #ifdef PP_DEBUG
  virtual void dbDump(FILE* out)const;
  #endif


  /*
   * Real work.
   */

  void Link(){link_count++;}

> What about returning "this" here? Would allow for
> FooPointer = bar->Link ();
> Just an idea

  void Unlink(){if(!--link_count)delete this;}

  /*
     Descendents need virtual destructors.
     this should enforce that when using egcs.
  virtual ~ppLinkCounted(){}
  */
};



/**
  Exception Handling.  In the light of the fact that recent versions
  of gcc (egcs, 2.8 etc.) seem to have sane C++ support, we believe
  we can safely use this.

  We can even use RTTI if we want.  ?Should we?
**/

//
// Base exception class.
// Later we might feel the need to make this thing a bit cleverer.

> Ok, I changed that stuff. Have a look at it and tell me if you don't agree

// Update: I tried to improve it - hope this is fine
//
class ppException
{
private:

	const char *_Origin;  // Where was it thrown?
	const char *_Type;   // What exception category is it in?
	const char *_Details; // What happened exactly?


public:

	ppException (void) :
		_Origin ("unknown"),
		_Type ("Error: Unknown PPlay Exception."),
		_Details ("unknown")
	{}

	// Every error of given type has a particular.
	const char *Type    (void) { return _Type;    }

	// These strings should be set on a per construction basis.
	// The constructors of derived exception classes need to set them:
	// ppSomeException::ppSomeException (const char *Org, const char *Det);
	const char *Origin  (void) { return _Origin;  };
	const char *Details (void) { return _Details; };

  //FIX we should perhaps have a "operator<<(ostream&)" function or the like
  //but the corporate will has not yet decided how we handle this.
  //(i.e. we haven't decided between iostream or stdio).

> I'd leave it out to keep the class as minimalistic as possible

};


#endif /*_PP_CPLUSPLUS*/

#endif /*_PenguinPlay_h*/
--------------------------------------------

Cu
	Christian
--

God is real (unless declared as integer).