Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Massacring C Pointers

wozniak.ca

Post reply on HN
Print view View original

11–20 of 300 posts

Re: Massacring C Pointers

#11

Post by _8j50 »

the_lurker wrote: post #4

Yesterday I encountered a similar program on a HN comment chain as shown in this link. I am genuinely confused as to why this program is bad. I am a student and I do not know the best practices regarding pointers, but it is how I would write a program to combine two strings. Can someone please elaborate why it is bad? Are their any good resources to fill gaps in my knowledge? Thanks in advance. Edit: Thank you guys f…

There are a few bugs in it. It returns something on the local functions stack (r) which gets torn down after return. It uses the insecure strcpy allowing the caller to stack overflow the destination. It increments t in the loop after dereferencing it which only changes the value pointed to by t. x and y are improperly initialized, it null terminates r without checking for x's value(again a buffer overflow) and there is no upper bound limit checking in the for loop(second for() parameter). I'm sure there are other bugs too but I haven't looked at it too closely.

But,yeah... It's a massacare.

Re: Massacring C Pointers

#12

Post by oddity »

the_lurker wrote: post #4

Yesterday I encountered a similar program on a HN comment chain as shown in this link. I am genuinely confused as to why this program is bad. I am a student and I do not know the best practices regarding pointers, but it is how I would write a program to combine two strings. Can someone please elaborate why it is bad? Are their any good resources to fill gaps in my knowledge? Thanks in advance. Edit: Thank you guys f…

From a purely safety-minded perspective, this function has a hidden bound of r[] past which the function becomes unsafe, but it does not check. Not only does it not check, the design of the function means that there is no possible way for it to check safely. s and t are both pointers to characters? How long are the strings they might represent supposed to be? Who knows? This code is incredibly reckless and it's dangerous to introduce students to such carelessly designed examples when first impressions will define how they program for a few years or so.

Ok, so it's reasonable to think that this is supposed to be a teaching example and that considering these concepts might be a bit too early in the process. This leads into the second problem and one that is more subjective: this code is incredibly dense and relies on enough quirks of C that it's almost never going to be clear to a beginner reader what they are supposed to take away from it. It's maybe useful as a quiz question on C syntax and semantics, but there are enough barriers to understanding what the code is supposed to do that the amount of explaining the text would need to do to describe what the code is doing is most likely prohibitively long. Instructive examples should be unambiguous in what they are trying to show, otherwise students will be confused and potentially conflate issues in a way that is difficult to untangle later.

Edit: Ha! I spent so long looking at the first half of the function I totally missed that it was returning r! So, not only does this code have minor issues here and there from its careless implementation, it has a fundamental flaw that, if it were to work, would do so only by accident. I can only imagine that a student might walk away from this example thinking that C functions can return arrays and possibly misunderstand scoping in C.

Re: Massacring C Pointers

#13

Post by bjoli »

I doubt books like these are as common today, but tutorials are everywhere. I don't know how often I have found scheme tutorials that teach a language I barely understand. Not dangerous maybe, but very weird nonetheless.

I see beginners writing code like that all the time, which makes me sad.

Re: Massacring C Pointers

#14

Post by [deleted] »

the_lurker wrote: post #4

Yesterday I encountered a similar program on a HN comment chain as shown in this link. I am genuinely confused as to why this program is bad. I am a student and I do not know the best practices regarding pointers, but it is how I would write a program to combine two strings. Can someone please elaborate why it is bad? Are their any good resources to fill gaps in my knowledge? Thanks in advance. Edit: Thank you guys f…

[deleted]

Re: Massacring C Pointers

#15

Post by [deleted] »

the_lurker wrote: post #4

Yesterday I encountered a similar program on a HN comment chain as shown in this link. I am genuinely confused as to why this program is bad. I am a student and I do not know the best practices regarding pointers, but it is how I would write a program to combine two strings. Can someone please elaborate why it is bad? Are their any good resources to fill gaps in my knowledge? Thanks in advance. Edit: Thank you guys f…

[deleted]

Re: Massacring C Pointers

#16

Post by saagarjha »

oddity wrote: post #12
the_lurker wrote: post #4

Yesterday I encountered a similar program on a HN comment chain as shown in this link. I am genuinely confused as to why this program is bad. I am a student and I do not know the best practices regarding pointers, but it is how I would write a program to combine two strings. Can someone please elaborate why it is bad? Are their any good resources to fill gaps in my knowledge? Thanks in advance. Edit: Thank you guys f…

From a purely safety-minded perspective, this function has a hidden bound of r[] past which the function becomes unsafe, but it does not check. Not only does it not check, the design of the function means that there is no possible way for it to check safely. s and t are both pointers to characters? How long are the strings they might represent supposed to be? Who knows? This code is incredibly reckless and it's dange…

I'm sorry, but I don't think you've quite gotten the reasons why it was lambasted:

> Not only does it not check, the design of the function means that there is no possible way for it to check safely. s and t are both pointers to characters? How long are the strings they might represent supposed to be?

strlen

> This leads into the second problem and one that is more subjective: this code is incredibly dense and relies on enough quirks of C that it's almost never going to be clear to a beginner reader what they are supposed to take away from it.

Most "nice" C functions are much terser than this.

Re: Massacring C Pointers

#17

Post by leodeid »

the_lurker wrote: post #4

Yesterday I encountered a similar program on a HN comment chain as shown in this link. I am genuinely confused as to why this program is bad. I am a student and I do not know the best practices regarding pointers, but it is how I would write a program to combine two strings. Can someone please elaborate why it is bad? Are their any good resources to fill gaps in my knowledge? Thanks in advance. Edit: Thank you guys f…

I haven't touched C in years, but here's my descending "wtf" list:

1. Returns pointer to stack-allocated data, which immediately becomes invalid. Instead, it should be using some sort of allocation (e.g. 'malloc'), or taking in a destination pointer.

2. 'r' is arbitrarily set with length 100. Smaller strings don't need all that space, and larger strings definitely will overrun.

3. The function signature is really awkward. Without any of the surrounding textbook content, I'm not sure what behavior is supposed to happen. At first, I expected something like 'strcat', which takes two char* and appends the second one to the first one. But that isn't happening here and instead it seems to require dynamic allocation. (Hiding allocations inside a function is generally kind of weird. Usually the caller should be responsible for passing in a handle to the destination.)

4. There's no sensical limit on the loop iteration. If the input 't' doesn't have a null terminator, this is going to throw a ton of garbage into the stack space (because 'r' is stack-allocated to a fixed size). And also maybe run for a really long time.

5. 'strcpy' should usually be replaced by 'strncpy', which performs the same function but also requires you to provide a limit ("copy this string, but at most 'n' bytes"). That prevents a class of exploitable errors known as "buffer overruns". I don't know when the 'n' string functions were added to C or became popular, though.

This is a teaching exercise, so the fact that this is implemented as a separate function instead of calling 'strcat' from doesn't seem like a big problem.

Re: Massacring C Pointers

#18

Post by saagarjha »

_8j50 wrote: post #11
the_lurker wrote: post #4

Yesterday I encountered a similar program on a HN comment chain as shown in this link. I am genuinely confused as to why this program is bad. I am a student and I do not know the best practices regarding pointers, but it is how I would write a program to combine two strings. Can someone please elaborate why it is bad? Are their any good resources to fill gaps in my knowledge? Thanks in advance. Edit: Thank you guys f…

There are a few bugs in it. It returns something on the local functions stack (r) which gets torn down after return. It uses the insecure strcpy allowing the caller to stack overflow the destination. It increments t in the loop after dereferencing it which only changes the value pointed to by t. x and y are improperly initialized, it null terminates r without checking for x's value(again a buffer overflow) and there…

> x and y are improperly initialized

How so?

Re: Massacring C Pointers

#19

Post by oldcynic »

Make sure to click the link at the end of the article for code samples with potential for Segmentation fault:core dumped in 4 lines.

Even more concerning is the book seems to have some positive reviews on Amazon(!), and just one shredding it.

Re: Massacring C Pointers

#20

Post by saagarjha »

leodeid wrote: post #17
the_lurker wrote: post #4

Yesterday I encountered a similar program on a HN comment chain as shown in this link. I am genuinely confused as to why this program is bad. I am a student and I do not know the best practices regarding pointers, but it is how I would write a program to combine two strings. Can someone please elaborate why it is bad? Are their any good resources to fill gaps in my knowledge? Thanks in advance. Edit: Thank you guys f…

I haven't touched C in years, but here's my descending "wtf" list: 1. Returns pointer to stack-allocated data, which immediately becomes invalid. Instead, it should be using some sort of allocation (e.g. 'malloc'), or taking in a destination pointer. 2. 'r' is arbitrarily set with length 100. Smaller strings don't need all that space, and larger strings definitely will overrun. 3. The function signature is really awk…

> If the input 't' doesn't have a null terminator

Then it's not a string.

Post reply on HN