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 = string;
}
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;
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;
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.
--

 

[Darrell Grainger]

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 is safe but is the string?

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

Why increment i? This could have just as easily been:

p = '\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;
j++;
}

Good. Might be better using:

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

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? 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(string)+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;
j++;
}

for(j = 0, i = from; i <= to; i++, j++)
p[j] = string;

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);

 

[Leor Zolman]

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 = string;

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;
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;
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

 

[Robert Bachmann]

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

 

[Joe Wright]

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 = string;
}
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(phone,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;
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;
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.

 

[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 = string;
}
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;
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;
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..


--

 

[Leor Zolman]

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

 

[Materialised]

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]

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 = string;

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;
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;
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]

Darrell Grainger wrote:

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?


--

 

[Leor Zolman]

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.

That's okay, it will work just fine in either style. As far as I'm
concerned, it is really six of one and a half dozen of the other; putting
it in is "unnecessary", leaving it out is inconsistent with other idiomatic
uses of sizeof. What can you do.
-leor

 

[Materialised]

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.

<snip>
Please ignore my ignorance, but if I had the string:

chat blah[] = "abcdef";

Why would I need to reference a negative value of the string?



--

 

[Richard Bos]

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

You're wrong in assuming that it is guaranteed. Of course, the same
thing could be said about memory that _is_ explicitly free()d, but
you're just ever so slightly more wrong about programs that do not clean
up after their own butts like their mama (i.e., you!) taught them to.

Besides, you may be wrong in assuming that this part of the code is
always going to stand alone. If it is ever incorporated in a larger
program, which, granted, is unlikely for a test program but rather more
plausible for real code, you'll be glad that you don't have to stop it
from leaking code then, when you have so many other compatibility issues
to think of.

Richard

 

[August Derleth]

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

Yes. That assumption is not portable and can cause problems on some
systems.

 

[Martin Dickopp]

Darrell Grainger wrote:

I usually use the term "memory leak" to refer to memory which can no
longer be accessed while the program runs. That's not the case here;
there are valid pointers to the allocated memory blocks until `main'
returns.

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

Neither behavior is guaranteed, but manually freeing memory at the end
of the program has advantages as well as disadvantages. If you search
the archives, you'll see that there is no consensus in this group about
this question.

Martin

 

[Nick Keighley]

Darrell Grainger wrote:

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

you are correct, but it is good practice for software to clean up behind
itself. Not all programs have "ends", they are supposed to run pretty well
continuously. Consider the joy that would greet this error message:-

"airbus.c line 5211: Assertion failed: malloced_val != NULL"



--
Nick Keighley

-pedantic
This option is not intended to be useful; it exists only to satisfy
pedants who would otherwise claim that GNU CC fails to support the
ANSI standard.
(Using and Porting GNU CC)

 

[Richard Bos]

Neither behavior is guaranteed, but manually freeing memory at the end
of the program has advantages as well as disadvantages.

Hmmm... I can't really see any disadvantages here - except the extra
typing, which is insignificant.

Richard

 

[Nick Keighley]

Robert Bachmann wrote:

Please ignore my ignorance, but if I had the string:

chat blah[] = "abcdef";

Why would I need to reference a negative value of the string?

quite. That's why he's suggesting you don't allow such calls. Another way
would be to check for values less than zero inside the function. Perhaps
using assert().


--
Nick Keighley

We recommend, rather, that users take advantage of the extensions of
GNU C and disregard the limitations of other compilers. Aside from
certain supercomputers and obsolete small machines, there is less
and less reason ever to use any other C compiler other than for
bootstrapping GNU CC.
(Using and Porting GNU CC)

 

[Alex]

Hmmm... I can't really see any disadvantages here - except the extra
typing, which is insignificant.

The only one I know of (and it's OT) is that calls to free() are often
relatively expensive.

 

[Richard Bos]

The only one I know of (and it's OT) is that calls to free() are often
relatively expensive.

Possibly, and possibly not; in any case, at the end of the program it is
hardly likely to matter a lot, is it?

Richard