problem in padding a string

[smithss]

Hi,

so i wrote a few of the programs on my own after all the help previosuly but i've got stuck in this one here. The point is the make the string alteast 'n' characters long by padding spaces to its right.

#include <iostream>

using namespace std;

void padRight(char *&a, int n)
{
	int diff = strlen(a) - n;

	if(diff >= 0)
	{
		return;
	}
	else
	{
		char *temp = a;
		//cout << temp << endl;
		a = (char *)malloc(n);
		memset(a,' ',n);
		while (*a++ = *temp++);
		//cout << a << endl;
	}
}

int main()
{
	char p[] = "smiths";
	cout << "before padding " << strlen(p) << " " << p << endl;
	char *a = p;
	padRight(a,10);
	cout << "after padding " << strlen(a) << " " << a << endl;
};

output:
>
before padding 6 smiths
after padding 10 _MONETm

Please point me to the errors with explanations.

[Ancient Dragon]

allocate and top the string first then reset pointer a

char* temp  = new char[n];
    char* p = temp;
    memset(temp,' ',n);
    temp[n-1] = 0; //null  terminate
    memcpy(temp, a, strlen(a));
    a = temp;

[smithss]

Hi,

Couple of doubts

1> what was the error in my code? Why was in not copying the temp string back to a?
2> i realize that i'm allocating memory and it should be freed, where can i do that? In Main?

editing:

3> the output is slightly off

>
before padding 6 smiths
after padding 9 smiths

so even though you are setting temp[n-1] = 0, which seems correct because arrays start from 0, why is the strlen giving length as '9'?

Thanks

[smithss]

ok i got the 3rd point. its because strlen doesn't count the null terminator hence we're getting a length of 9. The output is perfectly alright then :). Awaiting answers to 1 & 2.

[Murtan]

Did you attempt to walk through what your code does in your head?

Here's your code with my comments included:

// Assign a to temp
char *temp = a;
// temp and a both point to the original buffer

// Assign a to point to a new buffer n characters long
a = (char *)malloc(n);
// a points to the new buffer
// ? did you allocate space for the '\0' at the end of the string?

// Initialize the new space pointed to by a to all spaces
memset(a,' ',n);
// a points to the new buffer of spaces

// This one line does a bunch...
// each iteration copies one character from the original string to the new buffer
// both pointers are advanced  to point to the next character
// The loop continues until the '\0' is copied
while (*a++ = *temp++);
//cout << a << endl;

// Now the loop is done:
// temp points to one character beyond the '\0' in the original buffer
// a points to one character beyond the '\0' in the new buffer

// as a is a reference parameter, the pointer back in the main loop also points one beyond the '\0' in the new buffer.

Does that help with 1 & 2?

[smithss]

Did you attempt to walk through what your code does in your head?

Here's your code with my comments included:

// Assign a to temp
char *temp = a;
// temp and a both point to the original buffer

// Assign a to point to a new buffer n characters long
a = (char *)malloc(n);
// a points to the new buffer
// ? did you allocate space for the '\0' at the end of the string?

// Initialize the new space pointed to by a to all spaces
memset(a,' ',n);
// a points to the new buffer of spaces

// This one line does a bunch...
// each iteration copies one character from the original string to the new buffer
// both pointers are advanced  to point to the next character
// The loop continues until the '\0' is copied
while (*a++ = *temp++);
//cout << a << endl;

// Now the loop is done:
// temp points to one character beyond the '\0' in the original buffer
// a points to one character beyond the '\0' in the new buffer

// as a is a reference parameter, the pointer back in the main loop also points one beyond the '\0' in the new buffer.

Does that help with 1 & 2?

well i did and your comments tell me that i was thinking right till the last step of copying. That was bang-on target, both the pointers are pointing to the end of strings and i will have to rewind them if i have to cout the correct string, hence using AD's method would be much easier.

that satisfies 1 but 2 was about freeing the memory, where should i be doing that? i have one more doubt but that's more generic and i'll ask that later :)

[Murtan]

You'll have to free the memory back in main when you're done with it.

I would almost prefer that you pass an output buffer to the padRight function that is guaranteed to have enough space for the padded string. That would allow you to use either a buffer from the stack (that does not need to be allocated / deallocated) or a buffer from the heap (with new / delete) at the discretion of the calling code. It would then be the responsibility of the calling code to manage the allocation.

In your current implementation, you only allocate if the string needs padding, so sometimes 'a' will point to allocated memory and sometimes it will not. That makes management of the allocation much harder. The calling function must be aware of the internal implementation of padRight, generally not a good design.

[smithss]

I'm right now in that state of confusion that comes right before the moment of clarity. Please bear with me.

when i wrote mystrcpy i did

void mystrcpy(char *a,char *b)
{
	while (*b++ = *a++);
}
int main()
{
	char *s = "rajats";
	char d[8] = "rajatch";
	mystrcpy(s,d);
	cout << d << endl;
};

this also seems pass by value to me now. so 'd' is being passed by value then how is that when i copy 'a'-'b' the effect is visible in main? shouldn't i be passing the reference to the pointer here as well?

2> in my padRight program, 'a' is a pointer to a static array, then when i assign 'temp' to this pointer does the array size change? at 'a = temp;' step.

[Murtan]

You have to distinguish between the pointers and the data that they point to.

The mystrcpy works by changing its copy of the pointers, updating what the pointers point to.

When you call mystrcpy(s, d) s will not change as it is not a reference, and d will not change as it is also not a reference, but the contets of where d points (to the bufffer in main) IS updated from the input string.

In pad right, if you made a call like the following: void padRight(char const * src, int minwidth, char * dest) where minwidth is the minimum desired width and dest is a pointer to memory large enough to hold the greater of strlen(src) + 1 and minwidth + 1.

Then you would ALWAYS copy to dest and if strlen(src) was < minwidth, you would append spaces to dest until strlen(dest) >= minwidth.

If you wanted to be even safer, you could use a call like: void padRight(char const * src, int minwidth, char * dest, size_t destsize) and then your code could validate that dest (destsize) is large enough to hold the padded string.

[smithss]

so the way i am calling 'padRight', where there's no destination pointer there can be issues because 'a' is not big enough to hold the padded string. when i say "a = temp".

The first point is clear now. I think i can differentiate between changing the contents and changing the pointer now. thanks a lot.

one final qs. if the main function of 'padRight' is written as

int main()
{
	char *p = "smiths";
	cout << "before padding " << strlen(p) << " " << p << endl;
	//char *a = p;
	padRight(p,10);
	cout << "after padding " << strlen(p) << " " << p << endl;
};

I'm using 'char *p', this is a read-only memory but it still allows it to be changed. How does that happen?

[kbshibukumar]

Hi,

so i wrote a few of the programs on my own after all the help previosuly but i've got stuck in this one here. The point is the make the string alteast 'n' characters long by padding spaces to its right.

#include <iostream>

using namespace std;

void padRight(char *&a, int n)
{
	int diff = strlen(a) - n;

	if(diff >= 0)
	{
		return;
	}
	else
	{
		char *temp = a;
		//cout << temp << endl;
		a = (char *)malloc(n);
		memset(a,' ',n);
		while (*a++ = *temp++);
		//cout << a << endl;
	}
}

int main()
{
	char p[] = "smiths";
	cout << "before padding " << strlen(p) << " " << p << endl;
	char *a = p;
	padRight(a,10);
	cout << "after padding " << strlen(a) << " " << a << endl;
};

output:
>
before padding 6 smiths
after padding 10 _MONETm

Please point me to the errors with explanations.

I wonder when the loop while (*a++ = *temp++) will end. Also, as you have done an a++, the beginning of the string is altered.
I suggest using the following code.

int i = 0;
      while (i < strlen(temp))
      {
         a[i] = temp[i];
         ++i;
      }
      *(a + n) = 0; // To make it a string

Use of std::strings etc? No?

[smithss]

Use of std::strings etc? No?

No, not yet. The assignment is primarily to help us understand char arrays/pointers and their manipulation. I'm sure everyone takes this dusty road before enjoying the comfort of std::strings class :)

[smithss]

Ok i'm marking it solved. The original problem was solved i guess i'll clear the other doubts as i go along. Thanks a lot for your help, i could write all other programs without any issues, atleast one assignment is done !!! On to #2 .. :)