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

Sv: Sv: I'm back



>>I also think it would be a good idea to define this macro:
>>
>>#define PP_ASSERT_ARG(EXP) \
>>    if (!(EXP)) \
>>    { \
>>        ppWarning("Invalid argument"); \
>>        ppAssert(false); \
>>    }
>
>That would print:
>WARNING: In function "foo" at foofile: 123 : Invalid Argument
>ERROR (I): In function "foo" at foofile: 124 : Assertion false failed
>
>somehow weird ;)
>Calling ppInternalError () instead of ppWarning () and ppAssert () would be
>better.
>
Yeah.

>But in any case this is only useful for internal code (that can't be called
>directly by the user), and for that a simple ppAssert (Arg == valid); is
>fine.
>
I looked through the code, and realised we have a somewhat differing view of
how to check for arguments.

I consider it a bug to pass an invalid argument, you seem not to. Let me
give an example:

 int ppfFlush (ppFILE *stream) // FIX: make threadsafe
 {
  if (stream == 0)
  {
   ppWarning ("Invalid Arguments");
   return EOF;
  }
...
 }

This function is coded in suchs a way as to expect a stream value of 0. Its
completely normal behavior, and is handeld as suchs.

The way I would have coded the same function is like this: (using the
proposed ppAssertN to not evaluate the assert in release builds)

 int ppfFlush (ppFILE *stream) // FIX: make threadsafe
 {
  ppAssertN(stream != 0);
...
 }

or:

 int ppfFlush (ppFILE *stream) // FIX: make threadsafe
 {
#ifdef DEBUG
  if (stream != 0)
     ppInternalError("Invalid argument");
#endif
...
 }

This means that passing 0 as an argument is a bug, and therefore should not
be considered in release builds. Its simply not part of ppfFlush()'s
specification that it should handle parameter values of 0.

>>Other errors messages could be "Unknown internal PenguinPlay error",
>>"Unknown internal PenguinSound error" etc.
>
>I'd only use the generic PPlay message here - after all the file name and
>function in which it originated are printed as well.
>
Agreed.

>>"Memory corrupted or bug".
>
>Uh, don't know about the last one...
>
:)

Ok, perhaps just "Memory corrupted".