What is wrong with this feature?

I have a problem today. He had a method, and I need to find a problem in this function. The purpose of this function is to add a new line to the transmitted line. Below is the code

char* appendNewLine(char* str){ int len = strlen(str); char buffer[1024]; strcpy(buffer, str); buffer[len] = '\n'; return buffer; } 

I have identified a problem with this method. His view is straight. This method has the potential of having an array index out of range. This is not my doubt. In java, I use '\ n' for a new line. (I am mainly a Java programmer; I have been working in C for many years). But I vaguely remember '\ n' means the termination designation for a string in C. Is this also a problem with this program?

Please inform.

+4
source share
13 answers

There are several problems in this code.

  • strlen and not strlent unless you have an odd library function.
  • You define a static buffer on the stack. This is a potential error (and security), since the line is later, you copy the line into it without checking the length. Possible solutions for this would be to allocate memory on the heap (a combination of strlen and malloc) or use strncpy and accept line trimming.
  • Adding \ n 'really solves the problem of adding a new line, but this creates an additional error in that the line is not currently terminated by zero. Solution: add '\ n' and '\ 0' to the null termination of the new line.
  • As already mentioned, you return a pointer to a local variable, this is a serious error and returns a damaged value in a short time.

To broaden your understanding of these issues, see which C-style strings may be from here . Also, teach yourself the difference between the variables allocated on the stack and the variables allocated on the heap.

EDITed: AndreyT is correct, the definition of the length is really

+9
source

No, '\ n' is a new line in c, as in Java (Java grabbed it from C). You have identified one problem: if the input string is longer than your buffer , you will write it to the end of the buffer . Worse, your return buffer; returns a memory address that is local to the function, and will cease to exist when the function exits.

+7
source

At first it is a function, not a program.

This function returns a pointer to a local variable. Such variables, as a rule, are created on the stack and are not available when the function exits.

Another problem is that the value passed is longer than 1024 characters; in this case strcpy() will write the last buffer. One solution is to allocate a new buffer in dynamic memory and return a pointer to this buffer. The size of the buffer should be len +2 (all characters + newline + \0 string terminator), but someone will have to free this buffer (and, possibly, the original buffer).

strlent() does not exist, it should be strlen() , but I suppose it's just a typo.

+5
source

This function returns a buffer, which is a local variable on the stack. Once the function returns the memory for the buffer, it can be reused for another purpose.

You need to allocate memory using malloc or similar if you intend to return it from a function.

There are other problems with the code: you are not sure if the buffer is large enough to contain the line you are trying to copy, and you wonโ€™t be sure that the line ends with a null terminator.

+3
source
Lines

C ends with "\ 0".
And since your goal is to add newLine, the following will do everything (you will get rid of copying the entire line to the buffer):

 char* appendNewLine(char* str){ while(*str != '\0') str++; //assumming the string ended with '\0' *str++ = '\n'; //assign and increment the pointer *str = '\0'; return str; //optional, you could also send 0 or 1, whether //it was successful or not } 

EDIT:
The line must have space to accommodate the extra "\ n", and since the OBJECTIVE object itself must be added, which means adding to the original, its safe line can contain at least char !!
But, if you donโ€™t want to accept anything,

 char* appendNewLine(char* str){ int length = strlen(str); char *newStr = (char *)malloc(1 + length); *(newStr + length) = '\n'; *(newStr + length + 1) = '\0'; return newStr; } 
+3
source

Add zero after the newline:

 buffer[len] = '\n'; buffer[len + 1] = 0; 
+2
source

The terminator for a string in C is '\ 0' not '\ n'. It stands only for a new line.

+2
source

Your program has at least two problems.

First, it seems to you that you want to build a string, but you never terminate it with zero.

Secondly, you return a pointer to a locally declared buffer. It does not make sense to do this.

+2
source

There are several problems in the code:

  • It can overflow the buffer since buffer hard-coded to accommodate only 1024 characters. Even worse, the buffer is not allocated even on the heap.
  • The newline character is actually operating system dependent. Strictly speaking, this is only \n on Unix, etc. On Windows and in a strict Internet protocol, this is, for example, \r\n .
  • The string returned by the function does not end with zero. This is most likely not what you need.

Also, given your background in Java, consider the following things:

  • Since you are working with C char* and non (immutable) Java strings, maybe you could add a new line in place?
  • Access to the array is no longer checked at runtime, so you should be VERY careful about going out of bounds. Make sure all buffers are sized appropriately.
  • The language does not come with standard automatic garbage collection, so if you prefer to allocate new buffers for string manipulations, make sure that you correctly manage your memory and do not leak everywhere.
+1
source
 char* appendNewLine(char* str){ int len = strlen(str); char buffer[1024]; strcpy(buffer, str); buffer[len] = '\n'; return buffer; } 

Another important issue is the buffer variable; its supposed to be a local stack variable. As soon as the function returns, it is destroyed from the stack. And returning a pointer to a buffer probably means that you are going to crash your process if you try to write by the returned pointer (the address of the buffer whose address is on the stack).

Use malloc instead

+1
source

I ignore the return of the local, as others have eloquently talked about this.

 int len = strlen(str); char buffer[1024]; ... buffer[len] = '\n'; 

If strlen (str)> 1024, then this sequence will write outside the declared buffer. Also, as noted, this (possibly) will not be terminated by zero.

To safely add a new line, if you have one,

 char buffer[1024]; strncpy(buffer, str, 1024); // truncate string if it is too long int len = strlen(buffer); if (len < 1022) { buffer[len] = '\n'; buffer[len + 1] = '\0'; } 

Note. If the line is too long, this leaves the truncated line WITHOUT a new line.

+1
source
Line

C must end with '\0' .

 buffer[len+1] = '\0'; 

You should dynamically allocate a buffer as a pointer to a len sized char:

 char *buffer = malloc(len*sizeof(char)); 
0
source

Perhaps \ n should be \ r \ n. Return + new line. This is what I always use and work for me.

-2
source

Source: https://habr.com/ru/post/1302601/


All Articles