help with generic function

The following is an attempt at a generic uppercase function, not my code but im trying to understand it. I dont understand the garbagestring function, whats it most likely to be and say i had a string "hello" and wanted to use this function to 'uppercase it' what would i need to change?

thanks!

Code:

---

// --------------------------- UCase() function

char *UCase(char *);

char *UCase(char *UCaseString)

{

        char *StrUCaseTmp;

        int StrUCaseLen;

        StrUCaseLen=strlen(UCaseString);

        StrUCaseTmp=garbagestring(StrUCaseLen);

        strncpy(StrUCaseTmp,UCaseString,StrUCaseLen);

        StrUCaseTmp=strupr(StrUCaseTmp);

        return StrUCaseTmp;

}

---

it looks to me like garbagestring is a pointer to some (semi)random data... IMO, that function looks like it could be quite dangerous if coded inproperly... I'm going to give you this code out of a recent project

This I coded like this only because they required char*'s, and I didn't have the option of using an external buffer:

Code:

---

char*capitalize(const char*in)
{
        std::string buf=in;
        for(register short unsigned int i=0;i<buf.length();i++)
        {
                buf[i]=toupper(buf[i]);
        }
        return const_cast<char*>(buf.c_str());
}

---

and here's how I'd suggest doing it:

Code:

---

std::string PXRUsers::capitalize(const std::string in)
{
        std::string retval=in;
        for(register unsigned int i=0;i<in.length();i++)
        {
                retval[i]=toupper(retval[i]);
        }
        return retval;
}

---

Quote:

---

Originally Posted by major_small

it looks to me like garbagestring is a pointer to some (semi)random data... IMO, that function looks like it could be quite dangerous if coded inproperly... I'm going to give you this code out of a recent project

This I coded like this only because they required char*'s, and I didn't have the option of using an external buffer:

Code:

---

char*capitalize(const char*in)
{
        std::string buf=in;
        for(register short unsigned int i=0;i<buf.length();i++)
        {
                buf[i]=toupper(buf[i]);
        }
        return const_cast<char*>(buf.c_str());
}

---

and here's how I'd suggest doing it:

Code:

---

std::string PXRUsers::capitalize(const std::string in)
{
        std::string retval=in;
        for(register unsigned int i=0;i<in.length();i++)
        {
                retval[i]=toupper(retval[i]);
        }
        return retval;
}

---

---

Thanks! your code looks fine but the thing is that i have loads of these functions, pretty much every function you can get , lcase, mid, split, instr, etc.. coded generically in a sense. The point was to take VB code and turn it into C++. Now the guy that wrote all these corresponding C++ functions really knows what he's doing, you can tell by the code. So basically your saying this code looks dodgy ? If i could understand te garbagestring and add a function for it i would be away.

Um ... the first of those will invoke undefined behaviour. std::string frees its buffer upon destruction, i.e. as the function exits. The return value points to invalid memory. That's not to mention the const cast.

Code:

---

return std::strcpy(new char[buf.length()+1], buf.c_str());

---

(Will invoke undefined behaviour on non-conforming (by not throwing) new implementations on out-of-memory. Is also inefficient.)

And why on earth short?

Code:

---

char *str2upper(char *input)
{
  size_t len = strlen(input);
  char *out = new char[len+1];
  std::transform(input, input + len, out, toupper);
  out[len] = 0;
  return out;
}

---

But I agree with major_small that using std::string is better.

Ok i found the garbagestring function. I put it all together. Can anybody make any sense of it?

Code:

---

#include <iostream>
#include <windows.h>

char *UCase(char *);
char *garbagestring(DWORD StrBytes);
void initgarbage(void);
int GarbagePos;
char *GarbageArray[0x4000];

__inline void AsmPointerFill(DWORD,DWORD,DWORD);
__inline void *MallocZero(size_t);

char *garbagestring(DWORD StrBytes)
{
        char *CurrentGarbage;

        if (GarbageArray[GarbagePos]) free((void *)GarbageArray[GarbagePos]);

        CurrentGarbage=(char *)MallocZero(StrBytes+1);

        GarbageArray[GarbagePos]=CurrentGarbage;

        GarbagePos=(GarbagePos+1)&0x3fff;

        return CurrentGarbage;
}

char *UCase(char *UCaseString)
{

        char *StrUCaseTmp;

        int StrUCaseLen;

        StrUCaseLen=strlen(UCaseString);

        StrUCaseTmp=garbagestring(StrUCaseLen);

        strncpy(StrUCaseTmp,UCaseString,StrUCaseLen);

        StrUCaseTmp=strupr(StrUCaseTmp);

        return StrUCaseTmp;

}


__inline void *MallocZero(size_t SizeToAlloc)
{
        DWORD AllocatedMem;
        AllocatedMem=(DWORD)malloc(SizeToAlloc);
        AsmPointerFill(AllocatedMem,0,SizeToAlloc);
        return((void *)AllocatedMem);
}


__inline void AsmPointerFill(DWORD MemToFill,DWORD FillDat,DWORD FillLen)
{
        _asm {
                push eax
                push ecx
                push edi
                mov edi,MemToFill
                mov ecx,FillLen
                mov eax,FillDat
                cld
                shr ecx,2
                rep stosd
                mov ecx,FillLen
                and ecx,3
                rep stosb
                pop edi
                pop ecx
                pop eax
        }
        return;
}

int main(){
   
 UCase("hello"); 
   
}

---

As garabgestring is always associated with strncopy and the format of strncopy is,

Code:

---

char *strncpy(char *dest, const char *src, size_t n);

---

an example being,

Code:

---

char str1[]= "To be or not to be";
  char str2[6];
  strncpy (str2,str1,5);

---

so its and temp array to hold the string, so why has this code got what its got basically ?

Could i just say

char garabgestring[];

?

Quote:

---

Originally Posted by CornedBee

Um ... the first of those will invoke undefined behaviour. std::string frees its buffer upon destruction, i.e. as the function exits. The return value points to invalid memory. That's not to mention the const cast.

---

oh I know that... I suppose I didn't put enough emphasis on not using that method... actually, I probably shouldn't have posted it in the first place, huh?

Why not something simple?

Code:

---


# include <string>
# include <algorithm>
using namespace std;

void upcase(string& s)
{
    std::transform(s.begin(), s.end(), s.begin(), toupper);
}

---

Whats the difference between char and char* ??

Also how can i declare a dynamic array i.e

garbagestring[]; That gives me an error telling me its undimensioned ?

Finally whats wrong with this,

Code:

---

#include <iostream>

using namespace std;
int main (){
 
                }

char *UCase(char *UCaseString)
{

        char *garbagestring[100]; // ideally redimension array
        //  garbagestring[strlen(UCaseString)???       
       
        char *StrUCaseTmp;

        int StrUCaseLen;

        StrUCaseLen=strlen(UCaseString);

        strncpy(garbagestring,UCaseString,StrUCaseLen);

        StrUCaseTmp=strupr(garbagestring);

        return StrUCaseTmp;

}

---

This gives me the error,

cannot convert `char**' to `char*' for argument `1' to `char* strncpy(char*, const char*, size_t)' at line,

strncpy(garbagestring,UCaseString,StrUCaseLen);.

thanks again for any help!

a char is a space to hold a character. a char* is a pointer to that space.

what you did was create an array of 100 pointers to memory. what you watned to do was create 1 pointer to an array of 100 spaces of memory. consider changing

Code:

---

char *garbagestring[100]; // ideally redimension array

---

to

Code:

---

char*garbagestring=new char[100];
//or probably better in your case
char garbagestring[100];

---

a few notes: I'd use the second method at this point, because the first method opens you up to memory leaks... which taste really bad. Another thing is I didn't look at much of your code... that just kinda stuck out to me

garbagestring() allocates a block of memory and stores a pointer to it in a (kind of) circular array of pointers. When the circle is complete, the function starts freeing memory again. This is supposed to free the caller of functions that use garbagestring from having to manage the memory returned by these functions, but I consider it a bad idea. It's too unpredictable.

Using a class like std::string that does the memory handling is definitely better.

I think i see what the code was doing, could be wrong but, i think it was creating an array of size strlen(UCaseString) in other words,

Code:

---

garbagestring[strlen(UCaseString];

---

Is there another way to set a char array to a specific size ?

I need to say,

Code:

---

char garbagestring[];

//code

garbagestring = garbagestring[strlen(UCaseString];

---

That's possible in C99, but not C++. You have to use new[] for that, like I did in my function.

Have you heard of "The wheel of bad ideas"? There is a local radio show, where they occasionally spin the wheel of bad ideas. It's kind of like Wheel of Fortune, except it doesn't land on a dollar value--it lands on a really bad idea of something to do, and then they go ahead and do it. It's pretty funny to listen to, but I would never want to do it myself. Food for thought.

Excellent, thanks for all the help.!

One last question. Are there any problems with saying,

Code:

---

return strupr(garbagestring);

---

instead of,

Code:

---

StrUCaseTmp=strupr(garbagestring);
        return StrUCaseTmp;

---

?

They amount to the same thing.

That is, assuming that StrUCaseTmp is a local variable.