-1

I'm a noob to C and I found my program acting very weird:

Here's the code:

#include <stdbool.h>
#include <string.h>
#include <conio.h>
#include <stdio.h>

char * p(char arg[] , char sym[] , int i , bool rv) {
    char head[i],w[i];
    strncpy(head,arg,i); head[i] = "\0";
    int l;
    for (l = 0 ; l <= (int)(strlen(sym) / i) ; l++) {
        strncpy(w,sym+l*i,i); w[i] = "\0";
        if (strcmp(head,w) == 0) {
            if (rv) { return head; } else {
                char v[strlen(arg) - i];
                strcpy(v,arg+i);
                v[strlen(arg)-i] = "\0";
                return v;
            };
        };
    };
    return arg;
}

int main() {
    printf(p("/parameter","+-\\/",1,false));
    getch();
}

The MAIN problem is that the returning value of the function is either a "string" of randomly generated codes or simply nothing.

It was expected to return / for return h; and parameter for return v;.

The other problem is that there is NO error found when I compile the program but bunch of warnings telling that the function returns address of local variable and assignment makes integer from pointer without a cast.

In the other hand, return arg; in very peaceful and doesn't gives out any error. (Try to change my codes in p("/parameter","+-\\/",1,false) if you don't believe it.) What have I done wrong?


Usage of the function:

p("argument_passed_to_check","symbols_accepted to_be_at_the_front","separate_for_each_i_characters","return_header_instead_of_parameter")

expected result:

p("-argue","/\\-",1,false) returns argue

p("/help","me",1,false) returns /help

p("/help","me",1,true) returns (null)

p("--parameter","--++",2,false) returns parameter

p("--parameter","--++",2,true) returns --


Summary for what am I asking help for:

  • Except for return arg, other returning parts is weird: return head; is giving out random characters ; return v; returns nothing at all. How can I let them work as the expected results?

  • Why are there those warnings?

  • 6
    Off-topic: Do you know what's even better than a comment describing the purpose of those short opaque variable names? Giving those variables longer *descriptive* names that make the comment unnecessary. – StoryTeller - Unslander Monica Dec 24 '18 at 12:40
  • 2
    `h` And `v` are local variables. They don’t exist when the function returns so don’t return them. – Mark Tolonen Dec 24 '18 at 12:46
  • 2
    Treat warnings as errors. Replace "\0" with '\0' – manoliar Dec 24 '18 at 12:55
  • Thanks, but may I ask @MarkTolonen does that mean I should make `h` and `v` global to solve the problem? –  Dec 24 '18 at 12:59
  • Lai Yan Hui, Review poor code `head[i] = "\0";` That code attempts to assign a pointer to a `char`. – chux - Reinstate Monica Dec 24 '18 at 13:10
  • Given the definition `char head[i]`, where do you think `head[i]` = ...;` puts the data assigned? – Andrew Henle Dec 24 '18 at 13:12
  • @chux Thanks but manoliar has told be about that. –  Dec 24 '18 at 13:15
  • @AndrewHenle Sorry I don't get your meaning... I'm pretty new to it, sorry. –  Dec 24 '18 at 13:17
  • How many elements are in `head[]`? What's the index of the first one? – Andrew Henle Dec 24 '18 at 13:20
  • `int i` of characters (elements) `;` index of the first one is 0. –  Dec 24 '18 at 13:33
  • @StoryTeller: One should not regard named objects as self-documenting. Names cannot, without being excessively long, state preconditions, postconditions, or other important information, especially if the conditions involve relations with other items. There is nothing wrong with simple names for objects used in simple familiar ways accompanied by explanatory notes and conditions such as `x < y` or `p` points to `n` characters terminated by a null. A function filled with numerous repetitions of `PointerToNCharactersTerminatedByANull[i] = PointerToNCharacterTerminatedByANull[j];` is not better. – Eric Postpischil Dec 24 '18 at 15:18
  • @EricPostpischil - You counter example extrapolates my comment much further than I intended. I suspect that's intentional to prove a point, but one must note that nowhere did I recommend the OP codify in names well known idioms (like what a string in C is). My recommendation is for the name to codify the purpose it serves in accomplishing the function's task, if a short name suffices, fantastic. In the OP's case however, their choice of 1 character names is ungood. As is evident by the severe detrimental effect deleting the comment had on the clarity of their posted code. – StoryTeller - Unslander Monica Dec 24 '18 at 15:26

1 Answers1

2
  1. Since head is defined as char head[i], its last element is head[i-1]. Attempting to access head[i] has behavior not defined by the C standard.

  2. Since w is defined as char w[i], its last element is w[i-1]. Attempting to access w[i] has behavior not defined by the C standard.

  3. Since v is defined as char v[strlen(arg) - i], its last element is v[strlen(arg) - i - 1]. Attempting to access v[strlen(arg) - 1] has behavior not defined by the C standard.

  4. Since w is defined inside a brace-enclosed block of statements without extern or static, it is has automatic storage duration associated with the block, so it exists only while the function is block. When the return statement is executed, w ceases to exist (in C’s abstract machine). The statement return w; attempts to return a pointer to the first element of w (because, in this use, an array is automatically converted to a pointer to its first element). When this return statement is executed, the pointer becomes invalid.

  5. Since v is defined inside a brace-enclosed block of statements without extern or static, it has automatic storage duration associated with the block, so v exists only while the statement is executing. When return v; is executed, execution of the block ends, and the returned pointer becomes invalid.

  6. head[i] is a character, but "\0" is a string containing one character, so head[i] = "\0"; is an improper assignment. The string will be converted to a pointer to its first element, resulting in an attempt to assign a pointer to a char. This is a constraint violation, and your compiler should produce a warning for it. The same problem occurs in w[i] = "\0"; and v[strlen(arg)-i] = "\0";. The proper code would be head[i] = '\0'; (once the size of head is fixed to include an element head[i]).

Remedies include:

  • Define each array to be large enough for all the elements to be written into it.
  • To return strings created inside a function, either dynamically allocate space for them (as with malloc), create the strings inside arrays provided by the caller, or use arrays with static storage duration. If you use the first option, dynamically created arrays, you should make provisions for the space to be released (as by the caller passing them to free when done with them). You should avoid using arrays with static storage duration, as it has limited and troublesome use (such as the fact that only one such array exists for each definition, yet a function may be called multiple times by callers that each want their own separate data).
Eric Postpischil
  • 195,579
  • 13
  • 168
  • 312
  • Thanks for the guide~ (^u^) –  Dec 24 '18 at 13:20
  • But I would like to ask why does `char head[i]` is available to return random characters? –  Dec 24 '18 at 13:20
  • 1
    @LaiYanHui: When you declare `char head[i]`, the C compiler sets aside `i` bytes of memory to be used for `head`. It is then your responsibility to use only those `i` bytes. When you try to use `head[i]`, which is outside the array, the compiler does not stop you. Often, this results in your program accessing memory outside the array set aside for `head`, and that can break your program in various ways. Sometimes, other results may occur, as optimization by the compiler may transform your program in unexpected ways, and any behavior not defined by the standard is permitted to have any result. – Eric Postpischil Dec 24 '18 at 13:26
  • Thanks you very much~ –  Dec 24 '18 at 13:33