Another Understanding Pointers Question

[Materialised]

Hi everyone,
I seen the post by Rob Morris, and thought that I would double check
that I was using pointers in the correct way. So I written the following
string functions to test. I know soem can be iumplimented using the
standard libary, but I just wanted to test writing my own functions.
They work ok, but I would like some feed back on any issues you can see
with them etc

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

char *left(char *string, int count)
{
char *p;
int i;
p = malloc((count * sizeof(char)+1) );
if(!p){
printf("Cannot allocate memory\n");
exit(1);
}
for(i = 0; i <= count -1; i++) {
p[i] = string[i];
}
p[i++] = '\0';

return(p);

}

char *right(char *string, int count)
{
char *p;
int len, i, j = 0;
p = malloc((count * sizeof(char)+1) );
if(!p){
printf("Cannot allocate memory\n");
exit(1);
}
len = strlen(string);
for(i = (len - count); i <= len; i++){
p[j] = string[i];
j++;
}

p[j++] = '\0';
return(p);

}

char *chreplace(char *string, int count, char rep)
{
char *p;
p = malloc((sizeof( string)+1));
if(!p){
printf("Cannot allocate memory\n");
exit(1);
}
count--;
strcpy(p, string);
p[count] = rep;

return(p);
}

char *section(char *string, int from, int to)
{
char *p;
int i, j = 0;

p = malloc(((to - from) * sizeof(char)+1) );
if(!p){
printf("Cannot allocate memory\n");
exit(1);
}
for( i = from; i <= to; i++) {
p[j] = string[i];
j++;
}
p[j++] = '\0';

return(p);
}
int main(void)
{
char blah[] = "abcdefghijklm" ;
char *test;
char *test2;
char *test3;
char *test4;

test = left(blah, 10);
test2 = right(blah, 10);
test3 = chreplace(blah, 2, 'Q');
test4 = section(blah, 4, 10);
puts(test);
puts(test2);
puts(test3);
puts(test4);
return 0;
}

Comments and improvements are welcome, flames to if appropriate.
--
------
Materialised

perl -e 'printf "%silto%c%sck%c codegurus%corg% c", "ma", 58, "mi", 64,
46, 10;'

[Leor Zolman]

On Mon, 10 May 2004 15:11:42 +0100, Materialised <Ma**********@p rivacy.net>
wrote:

Hi everyone,
I seen the post by Rob Morris, and thought that I would double check
that I was using pointers in the correct way. So I written the following
string functions to test. I know soem can be iumplimented using the
standard libary, but I just wanted to test writing my own functions.
They work ok, but I would like some feed back on any issues you can see
with them etc
Okay, I'll point out some things I see. I'm sure there's much else others
will catch.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

char *left(char *string, int count)
Since you're not modifying string, define it as pointer-to-const:

char *left(const char *string, int count)
(or, Dan Saks style)
char *left(char const *string, int count)

{
char *p;
int i;
p = malloc((count * sizeof(char)+1) );
I don't know if there's a group consensus or not on whether it is worth the
trouble to say 'sizeof(char)' when the value is always 1. Personally, I'd
leave it out for the sake of simplicity:
p = malloc(count + 1);

Except for one thing: you don't know if you'll need all of that space.
What if someone specifies count as 10000? Wouldn't you be better of
figuring out, first, how much space you're /really/ going to need and just
allocating that?
if(!p){
printf("Cannot allocate memory\n");
exit(1);
}
for(i = 0; i <= count -1; i++) {
p[i] = string[i];
Another problem if count is too big is you'll keep going right off the end
of the source array here (undefined behavior).
}
p[i++] = '\0';

return(p);

}

char *right(char *string, int count)
All the same issues as above (well, now if count is too big you'll be
trying to read out of memory /before/ string instead of after it...)
{
char *p;
int len, i, j = 0;
p = malloc((count * sizeof(char)+1) );
if(!p){
printf("Cannot allocate memory\n");
exit(1);
}
len = strlen(string);
for(i = (len - count); i <= len; i++){
p[j] = string[i];
j++;
}

p[j++] = '\0';
return(p);

}

char *chreplace(char *string, int count, char rep) blah blah const blah blah...{
char *p;
p = malloc((sizeof( string)+1));
if(!p){
printf("Cannot allocate memory\n");
exit(1);
}
count--;
strcpy(p, string);
p[count] = rep; No sanity checking on count (again).
return(p); don't need parens there. return is not a function.
}

char *section(char *string, int from, int to) const,
sanity checking of from/to
{
char *p;
int i, j = 0;

p = malloc(((to - from) * sizeof(char)+1) );
if(!p){
printf("Cannot allocate memory\n");
exit(1);
}
for( i = from; i <= to; i++) {
p[j] = string[i];
j++;
}
p[j++] = '\0';

return(p);
}
int main(void)
{
char blah[] = "abcdefghijklm" ;
char *test;
char *test2;
char *test3;
char *test4;

test = left(blah, 10);
test2 = right(blah, 10);
test3 = chreplace(blah, 2, 'Q');
test4 = section(blah, 4, 10);
puts(test);
puts(test2);
puts(test3);
puts(test4);
return 0;
}

Comments and improvements are welcome, flames to if appropriate.

That should give you a bit to work on...
-leor
--
Leor Zolman --- BD Software --- www.bdsoft.com
On-Site Training in C/C++, Java, Perl and Unix
C++ users: download BD Software's free STL Error Message Decryptor at:
www.bdsoft.com/tools/stlfilt.html

[Darrell Grainger]

On Mon, 10 May 2004, Materialised wrote:

Hi everyone,
I seen the post by Rob Morris, and thought that I would double check
that I was using pointers in the correct way. So I written the following
string functions to test. I know soem can be iumplimented using the
standard libary, but I just wanted to test writing my own functions.
They work ok, but I would like some feed back on any issues you can see
with them etc

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

char *left(char *string, int count)
{
char *p;
int i;
It is a good habit to check your inputs. What if string == NULL? what if
count > strlen(string)? You can actually check this as you copy the string
over to p.
p = malloc((count * sizeof(char)+1) );
The value of sizeof(char) is always 1. This can be simplified to:

p = malloc(count+1) ;
if(!p){
printf("Cannot allocate memory\n");
exit(1);
}
I would be less likely to use a function that exits on failure. You have
taken away all control from the calling function. What if I had created a
lot of data then called your function? I'd be very annoyed to lose all
that data. If you returned NULL instead then I have the option to
continue, save and quit or just quit.
for(i = 0; i <= count -1; i++) {
I just like to see:

for(i = 0; i < count; i++) {

it seems more natural. Additionally, what if count > strlen(string)? The
p[i] is safe but is the string[i]?
p[i] = string[i];
}
p[i++] = '\0';
Why increment i? This could have just as easily been:

p[i] = '\0';
return(p);
}
As far as pointer usage goes, this to good. Some comments on the code
would be good. When I saw the count+1 in the malloc I was wondering why
the +1. I assumed it was for the null character but without comments I
couldn't be sure. As someone using your function I'd have to read and
understand the code to be sure I was using it correctly. Having a
commented at the top explaining usage would be good.
char *right(char *string, int count)
{
char *p;
int len, i, j = 0;
Same as previous function. Check your inputs.
p = malloc((count * sizeof(char)+1) );
Same as previous function.
if(!p){
printf("Cannot allocate memory\n");
exit(1);
}
Same as previous function.
len = strlen(string);
for(i = (len - count); i <= len; i++){
p[j] = string[i];
j++;
}
Good. Might be better using:

for(j = 0; i = (len - count); j < count; i++, j++)
p[j] = string[i];

First, I initialize j here. This way I'm sure I initialized it and no code
between the top of the function and here changed it.

Second, using the j index to exit the loop just seems right to me. This is
just a personal preference though.

Third, use the comma operator and make the code a little shorter. I like
to put as much on one screen as I can without losing readability.

Finally, we are sure that p[j] is safe. What about string[i]? You might
have an array out of bounds there as well.
p[j++] = '\0';
Again, why increment j?
return(p);
}
All-in-all, a good use of pointers.
char *chreplace(char *string, int count, char rep)
{
char *p;
Check the inputs.
p = malloc((sizeof( string)+1));
First serious mistake. The sizeof(string) will be the sizeof(char*). You
don't want the size of a pointer. You want the size of the string being
pointed to. This should be:

p = malloc(strlen(s tring)+1);
if(!p){
printf("Cannot allocate memory\n");
exit(1);
}
Same thing as previous functions.
count--;
Not immediately obvious why you are decrementing count. I can read your
code and figure out that the count parameter is 1-indexed and you are
decrementing it to make it 0-indexed but I should have to read your code
to figure that out.
strcpy(p, string);
p[count] = rep;
You could actually combine two lines and have:

p[--count] = rep;
return(p);
}
I like the fact that even of the user passing a literal string this
function still works.
char *section(char *string, int from, int to)
{
char *p;
int i, j = 0;
Check the inputs.
p = malloc(((to - from) * sizeof(char)+1) );
The sizeof(char) is redundant. What if to-from < 0? It could happen. Check
the inputs.
if(!p){
printf("Cannot allocate memory\n");
exit(1);
}
Same as previous functions.
for( i = from; i <= to; i++) {
p[j] = string[i];
j++;
}
for(j = 0, i = from; i <= to; i++, j++)
p[j] = string[i];
p[j++] = '\0';
Why increment j?
return(p);
}
Not bad but still had one serious mistake.
int main(void)
{
char blah[] = "abcdefghijklm" ;
char *test;
char *test2;
char *test3;
char *test4;

test = left(blah, 10);
test2 = right(blah, 10);
test3 = chreplace(blah, 2, 'Q');
test4 = section(blah, 4, 10);
puts(test);
puts(test2);
puts(test3);
puts(test4);
Lots of allocating of memory in the functions but absolutely not call to
free(). That would be a memory leak.

free(test4);
free(test3);
free(test2);
free(test);
return 0;
}
Comments and improvements are welcome, flames to if appropriate.
--
------
Materialised

perl -e 'printf "%silto%c%sck%c codegurus%corg% c", "ma", 58, "mi", 64,
46, 10;'

--
Send e-mail to: darrell at cs dot toronto dot edu
Don't send e-mail to vi************@ whitehouse.gov

[Robert Bachmann]

Materialised wrote:

Hi everyone,
I seen the post by Rob Morris, and thought that I would double check
that I was using pointers in the correct way. So I written the following
string functions to test. I know soem can be iumplimented using the
standard libary, but I just wanted to test writing my own functions.
They work ok, but I would like some feed back on any issues you can see
with them etc
Just a minor note on your use of int.
As far as I think your string functions
will never work with negative values.
So you may want to consider using size_t instead.
char *left(char *string, int count)

char *left(const char *string, size_t count)

etc.

Best regards,
Robert Bachmann
--
Ro************* @rbdev.net |) |)
http://rb.rbdev.net |\.|).

[Joe Wright]

Materialised wrote:

Hi everyone,
I seen the post by Rob Morris, and thought that I would double check
that I was using pointers in the correct way. So I written the following
string functions to test. I know soem can be iumplimented using the
standard libary, but I just wanted to test writing my own functions.
They work ok, but I would like some feed back on any issues you can see
with them etc

Good.
First, document the functions. Tell us the specification.
Second, functions which allocate memory, return a pointer and expect
the caller to free the memory make me nervous. Your first example
below will illustrate nicely.
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

char *left(char *string, int count)
{
char *p;
int i;
p = malloc((count * sizeof(char)+1) );
if(!p){
printf("Cannot allocate memory\n");
exit(1);
}
for(i = 0; i <= count -1; i++) {
p[i] = string[i];
}
p[i++] = '\0';

return(p);

} In dBASE, FoxPro, Clipper we have the very useful..

LEFT(<cString>, <nCount) -> cSubstring

...which we use like this..

phone = '7035551212' // a 10-character string
area = left(phone,3) // a 3-char string '703'
number = right(phone,4) // 4-char '1212'
exhg = left(right(phon e,7),3) // '555'

These are higher level languages than C. The targets of the
assignments are created by the assignment itself. Replicating this
in C will keep you awake nights. :-)


char *right(char *string, int count)
{
char *p;
int len, i, j = 0;
p = malloc((count * sizeof(char)+1) );
if(!p){
printf("Cannot allocate memory\n");
exit(1);
}
len = strlen(string);
for(i = (len - count); i <= len; i++){
p[j] = string[i];
j++;
}

p[j++] = '\0';
return(p);

}

char *chreplace(char *string, int count, char rep)
{
char *p;
p = malloc((sizeof( string)+1));
if(!p){
printf("Cannot allocate memory\n");
exit(1);
}
count--;
strcpy(p, string);
p[count] = rep;

return(p);
}

char *section(char *string, int from, int to)
{
char *p;
int i, j = 0;

p = malloc(((to - from) * sizeof(char)+1) );
if(!p){
printf("Cannot allocate memory\n");
exit(1);
}
for( i = from; i <= to; i++) {
p[j] = string[i];
j++;
}
p[j++] = '\0';

return(p);
}
int main(void)
{
char blah[] = "abcdefghijklm" ;
char *test;
char *test2;
char *test3;
char *test4;

test = left(blah, 10);
test2 = right(blah, 10);
test3 = chreplace(blah, 2, 'Q');
test4 = section(blah, 4, 10);
puts(test);
puts(test2);
puts(test3);
puts(test4);
return 0;
}

Comments and improvements are welcome, flames to if appropriate.

The 'sizeof(char)' thing is wrong and ugly. Take your pick.

Be aware and beware..

int i, to = 10;

for (i = 0; i <= to; ++i) {
body();
}
... will execute body() eleven times.
--
Joe Wright mailto:jo****** **@comcast.net
"Everything should be made as simple as possible, but not simpler."
--- Albert Einstein ---

[Materialised]

Materialised wrote:

Hi everyone,
I seen the post by Rob Morris, and thought that I would double check
that I was using pointers in the correct way. So I written the following
string functions to test. I know soem can be iumplimented using the
standard libary, but I just wanted to test writing my own functions.
They work ok, but I would like some feed back on any issues you can see
with them etc

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

char *left(char *string, int count)
{
char *p;
int i;
p = malloc((count * sizeof(char)+1) );
if(!p){
printf("Cannot allocate memory\n");
exit(1);
}
for(i = 0; i <= count -1; i++) {
p[i] = string[i];
}
p[i++] = '\0';

return(p);

}

char *right(char *string, int count)
{
char *p;
int len, i, j = 0;
p = malloc((count * sizeof(char)+1) );
if(!p){
printf("Cannot allocate memory\n");
exit(1);
}
len = strlen(string);
for(i = (len - count); i <= len; i++){
p[j] = string[i];
j++;
}

p[j++] = '\0';
return(p);

}

char *chreplace(char *string, int count, char rep)
{
char *p;
p = malloc((sizeof( string)+1));
if(!p){
printf("Cannot allocate memory\n");
exit(1);
}
count--;
strcpy(p, string);
p[count] = rep;

return(p);
}

char *section(char *string, int from, int to)
{
char *p;
int i, j = 0;

p = malloc(((to - from) * sizeof(char)+1) );
if(!p){
printf("Cannot allocate memory\n");
exit(1);
}
for( i = from; i <= to; i++) {
p[j] = string[i];
j++;
}
p[j++] = '\0';

return(p);
}
int main(void)
{
char blah[] = "abcdefghijklm" ;
char *test;
char *test2;
char *test3;
char *test4;

test = left(blah, 10);
test2 = right(blah, 10);
test3 = chreplace(blah, 2, 'Q');
test4 = section(blah, 4, 10);
puts(test);
puts(test2);
puts(test3);
puts(test4);
return 0;
}

Comments and improvements are welcome, flames to if appropriate.

Just a question,
I have taken all the people who replied comments to heart, and I am glad
for your feadback.
What I dont understand is what is wrong with sizeof(char)? True char may
be defined as 1 on most architectures. But does the way I have
referenced it require extra CPU useage or something?

Sorry if thats a dumb question..
--
------
Materialised

perl -e 'printf "%silto%c%sck%c codegurus%corg% c", "ma", 58, "mi", 64,
46, 10;'

[Leor Zolman]

On Tue, 11 May 2004 01:56:17 +0100, Materialised <Ma**********@p rivacy.net>
wrote:

I have taken all the people who replied comments to heart, and I am glad
for your feadback.
What I dont understand is what is wrong with sizeof(char)? True char may
be defined as 1 on most architectures. But does the way I have
referenced it require extra CPU useage or something?

Sorry if thats a dumb question..

Not dumb at all. The Standard spells it out as follows (6.5.3.4/3):

"When applied to an operand that has type char, unsigned char, or signed
char, (or a qualified version thereof) the result is 1."

So sizeof(char) is 1, period. Which makes writing it out a bit redundant,
that's all.
-leor
--
Leor Zolman --- BD Software --- www.bdsoft.com
On-Site Training in C/C++, Java, Perl and Unix
C++ users: download BD Software's free STL Error Message Decryptor at:
www.bdsoft.com/tools/stlfilt.html

[Materialised]

Leor Zolman wrote:

char, (or a qualified version thereof) the result is 1."

So sizeof(char) is 1, period. Which makes writing it out a bit redundant,
that's all.
-leor

So if my personal preference is to use sozeof(char) there will be no
portability issues with this?
Because tbh, (for me) it seems easier to remember.

--
------
Materialised

perl -e 'printf "%silto%c%sck%c codegurus%corg% c", "ma", 58, "mi", 64,
46, 10;'

[Materialised]

Leor Zolman wrote:

On Mon, 10 May 2004 15:11:42 +0100, Materialised <Ma**********@p rivacy.net>
wrote:

Hi everyone,
I seen the post by Rob Morris, and thought that I would double check
that I was using pointers in the correct way. So I written the following
string functions to test. I know soem can be iumplimented using the
standard libary, but I just wanted to test writing my own functions.
They work ok, but I would like some feed back on any issues you can see
with them etc

Okay, I'll point out some things I see. I'm sure there's much else others
will catch.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

char *left(char *string, int count)

Since you're not modifying string, define it as pointer-to-const:

char *left(const char *string, int count)
(or, Dan Saks style)
char *left(char const *string, int count)

{
char *p;
int i;
p = malloc((count * sizeof(char)+1) );

I don't know if there's a group consensus or not on whether it is worth the
trouble to say 'sizeof(char)' when the value is always 1. Personally, I'd
leave it out for the sake of simplicity:
p = malloc(count + 1);

Except for one thing: you don't know if you'll need all of that space.
What if someone specifies count as 10000? Wouldn't you be better of
figuring out, first, how much space you're /really/ going to need and just
allocating that?

if(!p){
printf("Cannot allocate memory\n");
exit(1);
}
for(i = 0; i <= count -1; i++) {
p[i] = string[i];

Another problem if count is too big is you'll keep going right off the end
of the source array here (undefined behavior).

}
p[i++] = '\0';

return(p);

}

char *right(char *string, int count)

All the same issues as above (well, now if count is too big you'll be
trying to read out of memory /before/ string instead of after it...)

{
char *p;
int len, i, j = 0;
p = malloc((count * sizeof(char)+1) );
if(!p){
printf("Cannot allocate memory\n");
exit(1);
}
len = strlen(string);
for(i = (len - count); i <= len; i++){
p[j] = string[i];
j++;
}

p[j++] = '\0';
return(p);

}

char *chreplace(char *string, int count, char rep)

blah blah const blah blah...

{
char *p;
p = malloc((sizeof( string)+1));
if(!p){
printf("Cannot allocate memory\n");
exit(1);
}
count--;
strcpy(p, string);
p[count] = rep;

No sanity checking on count (again).

return(p);

don't need parens there. return is not a function.

}

char *section(char *string, int from, int to)

const,
sanity checking of from/to

{
char *p;
int i, j = 0;

p = malloc(((to - from) * sizeof(char)+1) );
if(!p){
printf("Cannot allocate memory\n");
exit(1);
}
for( i = from; i <= to; i++) {
p[j] = string[i];
j++;
}
p[j++] = '\0';

return(p);
}
int main(void)
{
char blah[] = "abcdefghijklm" ;
char *test;
char *test2;
char *test3;
char *test4;

test = left(blah, 10);
test2 = right(blah, 10);
test3 = chreplace(blah, 2, 'Q');
test4 = section(blah, 4, 10);
puts(test);
puts(test2);
puts(test3);
puts(test4);
return 0;
}

Comments and improvements are welcome, flames to if appropriate.

That should give you a bit to work on...
-leor

Thats for your comments,
as regard to the sainity checking, thanks, the advice was well taken :)
I did think if including it actually, but as it was only a personal
example, I thought it wouldnt be a issue, but i understand if I wish to
be a good programmer, i should get into the habbit now.

--
------
Materialised

perl -e 'printf "%silto%c%sck%c codegurus%corg% c", "ma", 58, "mi", 64,
46, 10;'

[Materialised]

Darrell Grainger wrote:
<snip>

Lots of allocating of memory in the functions but absolutely not call to
free(). That would be a memory leak.

free(test4);
free(test3);
free(test2);
free(test);
return 0;
}

Am I wrong in assuming that memory is automatically free()'ed at the end
of the program?
--
------
Materialised

perl -e 'printf "%silto%c%sck%c codegurus%corg% c", "ma", 58, "mi", 64,
46, 10;'