0

I just want to know is it a good way of programming style.

I know what is happening in this piece of code. look for the first occurrence of href save it next_next and then look for the first occurrence of "}" and save it end_marker.

Here my question is end_marker[-1] = '\0'; is needed? Because strstr, upon successful completion, strstr() shall return a pointer to the located string or a null pointer if the string is not found.

I know the endmarker '\0' is for string but don't know is it good to index the array in the negative number?

Code:

char *end_marker;
char *next_next = strstr(links_ptr, "href");
 if (next_next != NULL) {
     next_next += 7;
     end_marker= strstr(next_next, "}");
     end_marker[-1] = '\0'; // :)
}

EDIT: links_ptr contains this data

 "links": [
        {
            "rel": "next",
            "href": "https://www.randomstuff.com/blabla"
        }
          ]
Improve this question
13
  • 1
    In C unlike Python or whatever your background is, accessing the -1 index isn't the last element. Commented Aug 7, 2017 at 9:04
  • 4
    It is well defined: stackoverflow.com/questions/3473675/… as E1[E2] is identical to (*((E1)+(E2))). Commented Aug 7, 2017 at 9:08
  • 1
    @NikolaiShalakin: Not necessarily. Commented Aug 7, 2017 at 9:16
  • 1
    What kind of equipment is this? Sounds scary. Commented Aug 7, 2017 at 9:22
  • 2
    Using a -1 as an index is perfectly correct, and your code is correct if all assumptions your code makes about the input string are fulfilled. But this is not robust code at all and next_next += 7; is rather hacky. Commented Aug 7, 2017 at 9:31

3 Answers 3

Reset to default
4

This usage of strstr assumes much about the input. Given input it doesn't expect, it can scan memory out of the string bounds, write to bad addresses, or try to dereference a null pointer.

If links_ptr is different - if it's part of user input or data downloaded on the internet - then it's a definite bug and security issue.

  • next_next += 7 assumes that strlen(next_next) >= 7. If the string is shorter you'll be scanning memory that doesn't belong to the string until the first '\0' or '}' is found.
  • if the previous scan finds '}' it will write '\0' to an unrelated address
  • if '}' isn't found, end_marker will be NULL and end_marker[-1] should crash
Improve this answer
Sign up to request clarification or add additional context in comments.

2 Comments

It uses rest-api for data exchange from the server then it search for the content what it downloaded, links_ptr is json buffer. If it fail to download the data from server.
If it's a server response then it falls under "external input". That's a crash or security vulnerability waiting to happen. Technically speaking, you can rely on validation to make sure the input meets expectations, but why would you? Fixing this is pretty trivial.
1

In C/C++, there's nothing evil in using a negative array index. In this way you are addressing the slot BEFORE the pointer represented by end_marker. However, you need to ensure that there's valid memory at this address.

Improve this answer

3 Comments

And how do you insure that ?
@TonyTannous - how do you ensure that any memory is valid?
This is ensured inside the program logic. When accessing end_marker[-1], the programmer is responsible for ensuring that this occurs only if end_marker is a pointer at least one slot after the base of the buffer he is using.
0

In this case it would be undefined behaviour, you should do

if (end_marker != NULL)
{
    end_marker[strlen(end_marker) - 1] = '\0';
}

Using negative number isnt good practice and you should do it. To do it you have to be sure there is still this array.

Improve this answer

3 Comments

Still not save. See this answer: stackoverflow.com/a/45543224/694576
Also whether the OP's code would provoke UB depends on the input, which is not given.
assert(*end_marker); is required.