Getting rid of warnings

OK, this "could" grow into a long thread (if someone does all the work - tehe )

Some projects build fine and work fine afterwards. Still they throw warnings around which i couldn't seem to understand nor be able to fix, so here is the first one.

warning: 'Examine' is deprecated (declared at /SDK/include/include_h/interfaces/dos.h:88)

the line in the code looks like this:

if (IDOS->Examine(pLock, fib) != DOSFALSE) {
if (FIB_IS_DRAWER(fib)) {
_bIsDirectory = true;
_pFileLock = IDOS->DupLock(pLock);
_bIsValid = (_pFileLock != 0);

I do know that this may be the "old" way to do this kind of thing, but i'm unable to change it, because i don't even know the old way, let alone any new way

If anyone is willing to tell me how i could get rid of those and, to a lesser degree, tell me exactly where the problem lies and why the warning shows up so i can add that to my learning curve

Thanks to all in advance

@Raziel

Check the autodoc for ExamineObject.

The command is very similar, and the structure defining the file attributes has changed from FileInfoBlock to ExamineData. Most of the field names are similar too, so the transition should be relatively easy.

Simon

@Raziel

Next question?

@Raziel

Somewhat more detailed answer:

BOOL ExDir( STRPTR dirname )
{
int32 success = FALSE;

APTR context = IDOS->ObtainDirContextTags( EX_StringNameInput,dirname,
EX_DoCurrentDir,TRUE, /* for recursive lock */
EX_DataFields,(EXF_NAME|EXF_LINK|EXF_TYPE),
TAG_END);
if( context )
{
struct ExamineData *dat;

while((dat = IDOS->ExamineDir(context)))
{

if( EXD_IS_FILE(dat) )
{
// Blah Blah
}
}

if( ERROR_NO_MORE_ENTRIES == IDOS->IoErr() )
{
success = TRUE; /* normal exit */
}
else
{
IDOS->PrintFault(IDOS->IoErr(),NULL); /* failure - why ? */
}
}
else
{
IDOS->PrintFault(IDOS->IoErr(),NULL); /* failure - why ? */
}

IDOS->ReleaseDirContext(context); /* NULL safe */
return(success);
}

@Ami603&Rigo

Thanks,i will try to incorporate them into the main program and see how far i can get

@tonyw

These i get a lot
warning: cast from type 'const char*' to type 'char*' casts away constness

@Raziel

Const char arrays are read only, if you use const char arrays in functions that expects char * read/write char pointer, then this warring will come.

If you how ever insist that const char arrays should be interpreted as char * in on this line the error will go away.



The “(char *)” part tells gcc to interpreted my_string as char * and prevents any compiler warnings.

@Raziel
Quote:
C is a strongly typed language. C++ is even more strongly typed. What you want to do with such languages is eliminate all type casting. There are some exceptions but the exceptions should always be by design. If you are adding (char*) anywhere in your code then your code is mostly likely broken.

There are a couple of wrinkles to using builtin strings in C and C++. First wrinkle is the languages do not define the sign of string literals (e.g. "this is a string literal.") so it can change from platform to platform. In AmigaOS, a 'char' type is implicitly unsigned. The second wrinkle is that string literals are implicitly of type const char*.

Armed with this you should be able to eliminate all your warnings without too much difficultly. You may encounter broken AmigaOS headers which still expect types like UBYTE that will require an explicit type cast and they are slowly being corrected.

Just don't fall into the trap of sprinkling your code with type casts until it compiles without warnings. Understand each and every type cast and why it is there and try to get rid of it. In this way, the compiler will be checking your code for errors long before it runs. This is a free design review and is a feature of strongly typed languages that you should take advantage of.

@ssolie

OK, so i won't touch them unless i'm willing to break portability

@Ami603

This may be the wrong way to understand how the AMiga way of doing things work, but as the new SDK changes some things, i'll simply watch others doing it and see if understand it

So, this piece of code is the old way, how would one do it the new way?

AmigaOSFilesystemNode::AmigaOSFilesystemNode(const Common::String &p) {
ENTER();

int offset = p.size();

//assert(offset > 0);

if (offset <= 0) {
debug(6, "Bad offset");
return;
}

_sPath = p;
_sDisplayName = ::lastPathComponent(_sPath);
_pFileLock = 0;
_bIsDirectory = false;

struct FileInfoBlock *fib = (struct FileInfoBlock *)IDOS->AllocDosObject(DOS_FIB, NULL);
if (!fib) {
debug(6, "FileInfoBlock is NULL");
LEAVE();
return;
}

// Check whether the node exists and if it is a directory
BPTR pLock = IDOS->Lock((STRPTR)_sPath.c_str(), SHARED_LOCK);
if (pLock) {
if (IDOS->Examine(pLock, fib) != DOSFALSE) {
if (FIB_IS_DRAWER(fib)) {
_bIsDirectory = true;
_pFileLock = IDOS->DupLock(pLock);
_bIsValid = (_pFileLock != 0);

// Add a trailing slash if it is needed
const char c = _sPath.lastChar();
if (c != '/' && c != ':')
_sPath += '/';
}
else {
//_bIsDirectory = false;
_bIsValid = true;
}
}

IDOS->UnLock(pLock);
}

IDOS->FreeDosObject(DOS_FIB, fib);
LEAVE();
}

Seems code pasting doesn't work here :-/

I'll have to add this is not my code

@Raziel

Pleas don't put opening braces at the end of the line, but on a lew line, indented from the previous line. The closing brace should line up with the opening brace so many lines down.

Without a little care like that, it's almost impossible to read the code and skim over it for formatting errors.

@Raziel



Note : quickly written at work did not tried to compile, so might be some syntax errors here and there, but general concept should be there.

EDIT: oh and code pasting works very well as you can see... At least when you are using the [ code ] tag

@tonyw

As i said, this is not my code, i just maintain (hahahahaha ) the port of the program that is build around it.
And as lately those warnings popped up i thought, i should maybe keep on with the flow to not eventually have a broken port because of all the updates in SDK.

Learning it the hard way by doing the errors of others is always the worst thing to do as a beginner, so thanks for pointing the braces thingie out

@abalaban

Thanks i will check if it works and will then keep on mugging you in dark corners of amigans.net, holding you down until you told me why it didn't work ... or why it worked

edit: One missing semicolon, but awesome, building, warning gone

Thank you

@Raziel
An easy way to get rid of depreciated warnings is to compile using the "-Wno-deprecated-declarations" switch... (Puts on flame retardant suit!)

@ChrisH

Depreciated functions are depreciated for a reason. The DOS functions, for example, have been replaced with 64bit counterparts, and developers are encouraged to use them in favour of the old 32bit ones, hence the warnings.

If everyone ignores the warnings, the 64bit functions might as well be removed again.

Simon

@ssolie

Quote:

Agreed, especially when the only difference is the "constness" of a variable. As annoying as it can be rewriting code to include const specifiers when you forgot to add them, those const specifiers help catch a lot of silly mistakes which can lead to very confusing bugs.

Hans

@Hans

Yes but sometimes the casts are required by the API itself : ever tried to pass a string into a TagItem structure ? And about constness, why on earth OpenClass() requires a STRPTR when a CONST_STRPTR would be enough ?

@Raziel

OK, here goes another one stopping me from compiling an other project.
It seems it's a double declaration of MiniGL/OpenGL in the source of the project, but how can i tell them that i already have MiniGL/OpenGL?

Of course, it could be something else aswell

C++ engines/grim/grim.o
In file included from /SDK/include/include_h/intuition/intuition.h:45,
from /SDK/local/common/include/mgl/context.h:27,
from /SDK/local/common/include/mgl/gl.h:72,
from /SDK/local/common/include/GL/gl.h:1915,
from /SDK/Local/newlib/include/SDL/SDL_opengl.h:44,
from ./engines/grim/gfx_opengl.h:34,
from engines/grim/grim.cpp:40:
/SDK/include/include_h/devices/inputevent.h:122: error: declaration of 'WORD IEPointerPixel::<anonymous struct>::v [1]'
/SDK/include/include_h/devices/inputevent.h:121: error: conflicts with previous declaration 'WORD IEPointerPixel::<anonymous struct>::v [0]'
/SDK/include/include_h/devices/inputevent.h:143: error: declaration of 'UWORD IEPointerTablet::<anonymous struct>::v [1]'
/SDK/include/include_h/devices/inputevent.h:142: error: conflicts with previous declaration 'UWORD IEPointerTablet::<anonymous struct>::v [0]'
/SDK/include/include_h/devices/inputevent.h:148: error: declaration of 'UWORD IEPointerTablet::<anonymous struct>::v [1]'
/SDK/include/include_h/devices/inputevent.h:147: error: conflicts with previous declaration 'UWORD IEPointerTablet::<anonymous struct>::v [0]'
In file included from /SDK/include/include_h/intuition/intuition.h:1636,
from /SDK/local/common/include/mgl/context.h:27,
from /SDK/local/common/include/mgl/gl.h:72,
from /SDK/local/common/include/GL/gl.h:1915,
from /SDK/Local/newlib/include/SDL/SDL_opengl.h:44,
from ./engines/grim/gfx_opengl.h:34,
from engines/grim/grim.cpp:40:
/SDK/include/include_h/intuition/screens.h:98: error: declaration of 'UWORD DrawInfo::<anonymous struct>::v [1]'
/SDK/include/include_h/intuition/screens.h:97: error: conflicts with previous declaration 'UWORD DrawInfo::<anonymous struct>::v [0]'
In file included from /SDK/local/common/include/mgl/gl.h:349,
from /SDK/local/common/include/GL/gl.h:1915,
from /SDK/Local/newlib/include/SDL/SDL_opengl.h:44,
from ./engines/grim/gfx_opengl.h:34,
from engines/grim/grim.cpp:40:
/SDK/local/common/include/mgl/minigl.h: In function 'void gluTessNormal(GLUtesselator*, GLdouble*, GLdouble*, GLdouble*)':
/SDK/local/common/include/mgl/minigl.h:1656: error: redeclaration of 'GLdouble* v'
/SDK/local/common/include/mgl/minigl.h:1656: error: 'GLdouble* v' previously declared here
/SDK/local/common/include/mgl/minigl.h:1656: error: redeclaration of 'GLdouble* v'
/SDK/local/common/include/mgl/minigl.h:1656: error: 'GLdouble* v' previously declared here

There are both tiny and opengl declared in the source


If i comment one of them out it stops with

engines/grim/grim.cpp: In member function 'virtual Common::Error Grim::GrimEngine::run()':
engines/grim/grim.cpp:346: error: expected type-specifier before 'GfxTinyGL'
engines/grim/grim.cpp:346: error: cannot convert 'int*' to 'Grim::GfxBase*' in assignment
engines/grim/grim.cpp:346: error: expected `;' before 'GfxTinyGL'

The code looks like this with the first g_driver line being line 346


Any ideas?

@abalaban

Quote:

I'm guessing that this was a mistake, and that OpenClass() really should have CONST_STRPTR. Sure there are times when it's necessary.

Hans

@Hans

It's impossible to know if input strings are read only or not.

If you choose to use const char * then also chosen to do a lot casting in your code, most functions take char *, even if string is not modified.

Because most functions take char *, and you need to cast to remove the warnings, the idea behind having different strings becomes useless, thats way I never use cont char * in my code.

I guess its ok if you lets say have int, and then you have function taking short, then ok know that data will be lost in the conversion, so you might select a better function or write some workaround for the problem other vise it useless.

@LiveForIt&Hans

Errm...yes, going back to my problem?

ahem, please?

@Raziel, sorry, but I cannot leave his last comment unanswered...

@LiveForIt

Quote:
It doesn't matter whether the input strings are read-only or not; specifying that an input parameter is const is a guarantee to the caller that you won't modify that parameter's data (string or something else). You can pass a char* as a const char* parameter without casting, but trying to use a const char* as a char requires a cast, because you could end up modifying a string that you previously said wasn't going to be modified.

A function writer knows if that function needs to modify a string or not, and therefore knows whether he/she should use the const specifier in the parameters. Failing to do so is a bug, because it makes it easier for programmers using that function to do something stupid.

Quote:
At least when you explicitly cast, you're thinking about the consequences of that action. If a function takes char* and you have a const char*, then you really should not assume that the function won't modify your string. Instead, you should copy the string to a new char* because you don't want the original to be modified.

Unfortunately lots of programmers forget to use const, or are just lazy, and cause trouble. As a result, there are plenty of functions out there that should have const parameters, but don't. This is poor programming practise, and shouldn't be copied, especially if you're only copying them in order to eliminate a few explicit casts.

The bottom line is, if your function/method accepts pointers/references to data that should remain unchanged, you should declare them to be const.

Hans