1

I am sorry if this question is obvious, or if I am making a simple logic mistake. I have searched for various ways of getting rid of the newline that comes from using fgets, but I continue running into problems while building. I think I am not understanding something properly and applying my "solution" incorrectly. I would like to be transparent and say that this is an assignment for school. Everything runs well except my output, which has unnecessary new lines.

The sole purpose of this function is to read names into an array of structs.

void initialize(FILE * ifp, Candidate * electionCandidates, int count)
{

for (int i = 0; i < count; i++)
{

    fgets (electionCandidates[i].name , 20 , ifp);

    if (electionCandidates[i].name[strlen(electionCandidates[i].name) - 1] == '\n')
    {
        electionCandidates[i].name[strlen(electionCandidates[i].name) - 1] = '\0';
    }   
} 
}

The following is displayed when I attempt to run: "Implicitly declaring library function "strlen" with type unsigned long (constant char*)"

rightymontand
  • 29
  • 1
  • 1
  • 2
  • is it your homework? – Kick Buttowski Sep 02 '14 at 04:34
  • 4
    Homework is not an issue. SO is here to help you learn. You keep doing your part, attempting your problems yourself, post the relevant code and any error messages, and people here are always happy to help. However, those that don't attempt their work first, and just want somebody else to do their homework -- usually are not as successful in getting help here, or in their careers beyond. – David C. Rankin Sep 02 '14 at 05:16
  • 4
    Your code is fine except for failing to `#include `. I have no idea why some of the other answers introduce a complicated solution. – M.M Sep 02 '14 at 06:03
  • `strlen(string) - positive_integer` mostly ever is a way into disaster and mostly never is necessary. – alk Sep 02 '14 at 07:01
  • A trick to remove the newline (either cute or terrible, depending how you look at it) is `strtok(electionCandidates[i].name, "\n");` – M.M Sep 02 '14 at 11:04

3 Answers3

2

1) No, not necessarily "obvious" - good question.

2) Yes, you want to use "strlen()".

3) It sounds like you forgot #include <string.h> to defined "strlen()".

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

char *trim (char *s) {
  int i = strlen(s)-1;
  if ((i > 0) && (s[i] == '\n'))
    s[i] = '\0';
  return s;
}
FoggyDay
  • 11,962
  • 4
  • 34
  • 48
  • Ha, yes, I did forget #include . Thank you for your quick reply, it provided good insight. I will try to fix my code. Thank you! – rightymontand Sep 02 '14 at 04:50
  • When overwriting the `newline` with the `null-terminating character`, you can either use the character representation of the null char `'\0'` as shown above, or you can use its integer value of `0`. Both are equivalent. (i.e. `s[i] = 0;`) So if you see either form, know both are equivalent. – David C. Rankin Sep 02 '14 at 05:20
  • 2
    `if ( i >= 0 ` does not protect against the cast of `strlen(s) == 0`. The `strlen` function returns a `size_t` which is an unsigned type, so assigning `(size_t)0 - 1` to an `int` causes implementation-defined behaviour. Instead, check either `*s != 0` or save `strlen(s)` without the `-1` before proceeding. – M.M Sep 02 '14 at 06:05
  • `strlen(string) - positive_integer` mostly ever is a way into disaster and mostly never is necessary. – alk Sep 02 '14 at 06:58
  • For a more **save** and even more flexible way to cut off a trailing possible line-end *sequence* (as it not necessarily always is one character only) you might like to take a look at this answer: http://stackoverflow.com/a/16000784/694576 – alk Sep 02 '14 at 07:04
0

Here is my take.

#include <stdio.h>
#include <string.h> // to fix the warning
...

// remove NL left by fgets, with protection
void trimnl (char *s) {
  char *pnl = s + strlen(s);
  if (*s && *--pnl == '\n')
    *pnl = 0;
}

void initialize(FILE* ifp, Candidate* electionCandidates, int count) {
for (int i = 0; i < count; i++) {
    fgets (electionCandidates[i].name, 20, ifp);
    trimnl(electionCandidates[i].name);
  } 
}

In my original version, the code read

  char *pnl = s + strlen(s) - 1;

This was criticised on the basis of signed arithmetic with unsigned values. However, the criticism does not apply because in this case the end result (if strlen = 0) is equivalent to:

  char *pnl = s - 1;

There is no unsigned arithmetic problem, but instead there is an Undefined Behaviour problem. That's why I changed the code.

david.pfx
  • 10,520
  • 3
  • 30
  • 63
  • 3
    It's a bit late to do `if (*s)` once you already pointed beyond the bounds of the string – M.M Sep 02 '14 at 06:03
  • @MattMcNabb: That pointer will never do any harm, but I accept that it is undefined behaviour. See edit. – david.pfx Sep 02 '14 at 10:00
  • @alk: Your comment doesn't apply here, but I changed the code anyway. – david.pfx Sep 02 '14 at 10:03
  • "*... doesn't apply here, but I changed ...*" should read "*... doesn't apply here anymore, because I changed ...*" shouldn't it? ;-) But whatsoever, cleaning up ... – alk Sep 02 '14 at 12:24
  • @alk: Sorry, what I meant was that your comment about `strlen()-int` did not apply to the specific code I had written because of the order of expression evaluation, but there was no point debating it because I changed the code to avoid the UB. See edit. – david.pfx Sep 02 '14 at 14:24
0

In the future, compile with -Wall to enable compilation warnings.

Depending on your compiler, you'll get useful advice on how to solve this and similar problems (which have been discussed in one answer elsewhere in this thread — compiling with Clang will warn you about the missing include, for instance).

Alex Reynolds
  • 95,983
  • 54
  • 240
  • 345