strcat

[Nobody/Anonymous]

hi, im trying to use the strcat function with char* and char. eg

char *text;

char letter;

strcat(text,letter);

why doesn't this work and what wouldi need todo to make it work?

[Anonymous]

strcat() does what the name suggests - it concatates strings, both operands must be of type char*

You might read the documentation rather than 'guess' the semantics and data types of a function. http://www.cppreference.com/stdstring/strcat.html

To do what you want you might do:

size_t len = strlen(text) ;
text[len] = letter ;
text[len+1] = '\0' ;

If you are doing this a lot, create a function:

char strAddChar( char string, char chr )
{
size_t len = strlen(string) ;
text[len] = chr ;
text[len+1] = '\0' ;

return string ;
}

Which does precisely what you expected strcat() to do.

Clifford

[Nobody/Anonymous]

There is a further error in your code:

char *text;
char letter;
strcat(text,letter);

You have not allocated any memory to *text.

strcat( char dest, char src ) requires that the destination string have sufficient space to store the concatenated string.

If you follow Cliffords example of simply appending a character to the end of a string, be aware that the string muse have enough space to accomodate the new character.

rr

[Anonymous]

Since nothing was initialised, I was assuming that these were just code fragments to indicatte data types.

Clifford

[Nobody/Anonymous]

Hi Everyone:

I'm guessing here, cause you didn't provide much information, but you probably want to be using strcpy(), not strcat(). You use both in a similar way, but as Clifford pointed out, you are not using it correctly anyway.

See Ya
Butch

[Nobody/Anonymous]

Well this is the whole function I was trying to get it working for if you want to see the context.

char ValidateUser(char Username,char Password)
{
char ReadBuffer[100];
char WStr = Username,ReadPass;
bool LoopMe = true;
FILE *UserFile;

strcat(WStr,".usr");

UserFile = fopen(WStr,"rb");

if(UserFile) // User Exists
{
    while(LoopMe)
    {
        fread(ReadBuffer,1,1,UserFile);
        if(ReadBuffer[0] == ':')LoopMe = false;
        else {
            strcat(ReadPass,ReadBuffer[0]); // LOOK HERE
        }
    }
if(ReadPass == Password)return VU_SUCCESS;
} else { // User Doesn't Exist
    return VU_INVALID_USER;
}
return VU_ERROR; // Shouldn't reach this

}

I was trying things like, strcat(ReadPass,(char *)ReadBuffer[0]);

I don't have a lot of experience with this function, keep posting tips if you want fellas!

A completely differant way would be nice, as Clifford suggested, but how would you do it differantly?

[Anonymous]

The code is not semantically correct, quite apart from the invalid use of strcat().

You have declared a 100 byte ReadBuffer, but you only ever use one byte, while you do not allocate any memory or initialise ReadPass at all, but write to it nonetheless!

The test if(ReadPass == Password) is incorrect, and will always be false, you are comparing two pointers - you cannot compare C style strings with the == operator because they are not themselves datatypes, they are arrays, you must use strcmp().

You append to the Username string, but you have no way of knowing whether sufficient space is allocated, or whether the caller of your function intended you to modify this string. Also appending to Username will cause the subsequent compare to always fail, unless the username in the file also has ".usr" on the end!

You have no test for end-of-file.

Personally I also have issues with the coding style - braces or no braces for single statement if/else clauses, one or the other but not both; more than one exit point in a function (one of which is unreachable); more than one variable declared in a single statement. char* is itself a type, so logically the * should cuddle the type identifier not the variable name in declarations.

Since the type of a literal constant or an enum is int, the function should probably return int for strict type agreement. Most 32bit processors handle ints more efficiently also.

Since you are reading character by character, you need not use strcat() at all, you can simply incrementally fill the buffer.

Even the strcmp() I suggested earlier is unnecessary, since you are already reading character by character, you simply terminate as soon as you read a mismatch (this is what strcmp does in any case). If you get to the ':' you exit with success.

I'd suggest the following (compiled but not tested):

int ValidateUser( char username, char password )
{
char filename[_MAX_FNAME] ;
int readchar ;
FILE* user_file ;
bool terminate = true;
int index ;
char status = VU_INVALID_USER ; // Initialise to invalid

// Create user file filename
strcpy( filename, username ) ;
strcat( filename, ".usr" ) ;

user_file = fopen( filename, "rb" ) ;

// If user file exists... 
if( user_file != 0 ) 
{ 
    for( index = 0; !terminate; index++ ) 
    { 
        readchar = fgetc( user_file ) ; 
        if( readchar == EOF )
        {
            // No delimiter found
            terminate = true ;
        }
        else if( readchar == ':' )
        {
            // Got as far as delimiter, so user name matches
            status = VU_SUCCESS ; 
            terminate = true ;
        }
        else if( readchar != username[index] )
        { 
            // Username mismatch
            terminate = true ;
        } 
    } 
}

return status ;

}

Clifford

[Nobody/Anonymous]

whats your problem, didn't get any xmas cards this year? personally i layout the function structure first and then add in luxuries such as error checking, but apparantly you decided this was the finished article. i only came for some help appending strings there was no need to bite my head off.

but thanks all the same

[Nobody/Anonymous]

oddly enough he did some things in his code that he complained about.

[Anonymous]

Now you've got me wasting my time trying to see where! Put me out of my misery - I am always willing to admit an error.

Clifford

[Nobody/Anonymous]

in that case you used char for the return type.

[Anonymous]

Interesting the compiler did not pick that one up. I'll have to check my compiler option settings - it was a gash project I use for building code posted to the forum. Sometimes I change the settings to match a user's, may have switched of -Werror -Wall.

It was a genuine error. I started with the OP's code, and incrementally adapted it. status should of course be declared int.

Clifford

[Anonymous]

...thanks, and well spotted BTW.

[Anonymous]

Tone is difficult to convey in text, and I did not mean to offend. It is also impossible to judge the audience - especially an anonymous one, so while some find such advice helpful others are ofended. If you post here regularly and consistently sign your posts (or create a named account), I will get to know your personality and experience and be able to respond appropriately. In the meantime, this is a public forum, that tends toward robust discourse, and you need to be prepared for all kinds of respnse and be less sensitive.

The coding style suggestions I prepended with 'Personally', to indicate that you can take it or leave it - but if you want others to read your code, making it readable helps!

You posted the code, which implicitly invites comment IMO - if you don't want it looked at, don't post it. The code you posted was incorrect in numerous ways; I am willing to bet that had that fact been ignored you would have been posting more questions later to fix those. Hopefully I have saved you the time and myself also.

Also because this is a public forum, from which others are also learning, it is hopefully useful to others to have mistakes corrected where possible, lest they be copied. It is always brave to post your code for all to see, but you must not see it as a personal attack or ridicule - the intent is to educate.

>>but thanks all the same

You're welcome, always a pleasure! ;-)

[Chris Severance]

> i only came for some help appending strings there was no need to bite my head off.

Help you got and it's nothing to complain about.

>personally i layout the function structure first and then add in luxuries such as error checking

When you get over all these noob problems, you'll learn to lay it all out at once.

>oddly enough he did some things in his code that he complained about.

I think Clifford is good enough that he can bend the rules. When you've not got string handling straight, you need to be consistant in whatever area you can.