Logic Error fixed in Solution

[SamiNasry]

The exercise asked to write a loop equivalent to the for loop that we used in getline() before in Chapter 1 without using && or ||.

The error that I fixed is that your solution doesn't have the same output as the old for loop for the same input. Your solution always add an extra character in the end of the string.

I avoided that problem by adding a break when the conditions are met.

[ohkimur]

@SamiNasry Great catch. Can you run the program before and after with some different types of strings so that you make sure it's actually valid? You can share them here so that we have a common reference. If everything is fine I'll merge your solution immediately.

[SamiNasry]

@ohkimur Yes I retried and It's valid. In both codes (the original getline() function that uses for(; ;) and your code) have a limit variable that basically stop the user from writing a lot of charachters in one string. I changed it to a small number (in my case 10) in both codes. So in your code if I type a 10 charchter long string (AZERTYUIOP) it doesn't remove anything which is incorrect becaude it should remove one charachter to leave space for '\n' in order to end up with a 10 charachters long string.

If I do the same thing in the original getline() function, this what I get.

And the explanation why this happens in your code it's because after you detect that I went more than (MAXLINE-1) and you change loop to 0 you don't get out of the while loop util you do s[i++] = c so you end up adding another extra character.

So the fix is adding the break to get out when the conditions are met.

[ohkimur]

@SamiNasry This solution works well. We can improve the code by merging all the conditions into one condition with logical OR. Then, we can have only one set for loop = 0 and only one break.

[ohkimur]

So, let's convert this:

if (i >= (MAXLINE - 1))
{
  loop = 0;
  break;
}
else if (c == '\n')
{
  loop = 0;
  break;
}
else if (c == EOF)
{
  loop = 0;
  break;
}

into something like this:

if (i >= (MAXLINE - 1) || c == '\n' || c == EOF)
{
  loop = 0;
}

I find this approach more compact and a bit easier to read. If you have a different opinion, please let me know.

@illustrofia mentions this is also an approach worth considering. I like it, but we're doing the same operation many times, which is a bit counterproductive. Also, having the same thing happening multiple times in multiple places will facilitate a pattern that might end up in divergence over time, especially in an environment where many devs are involved.

if (i >= (MAXLINE - 1))
{
  loop = 0;
}

if (c == '\n')
{
  loop = 0;
}

if (c == EOF)
{
  loop = 0;
}

[SamiNasry]

@ohkimur The more compact solution is good and easier to read but it doesn't fix the problem, you need to add the break after loop = 0, to get out of the while loop. So you change it into this :

if (i >= (MAXLINE - 1) || c == '\n' || c == EOF)
{
  loop = 0;
  break;
}

This fix is good and easier to read.

[ohkimur]

@SamiNasry I agree with you, except for the break, it doesn't make logical sense to me. I'll investigate it a bit and come back to you.

[SamiNasry]

Yes, try to get the original getline function and move the LIMIT to 10 and in your code move the MAXLINE to 10 and try to type strings with 10 charchters or more you will find the behavior is not the same for both codes. Without the break the code adds an extra charachter.

[ohkimur]

@SamiNasry I think I identified the potential issue. We're using different platforms. I'm using macOS, which is UNIX-like, and you're using Windows. For some reason, the Windows build behaves differently.

Here is the behavior on my machine:

If I put the break, it basically doesn't insert the new line char:

So, I basically propose to use this approach:

if (i >= (MAXLINE - 1) || c == '\n' || c == EOF)
{
  loop = 0;
}```

If it still doesn't print the last char on windows, please investigate why that might happen and let me know. I'm very curious what would be the best approach to handle these differences.

[SamiNasry]

You are right we have different behaviors. That's an interesting problem I will look into it further.
This is how it acts for me with the break

This is how it acts without the break

Anyways I will try to look into why it acts like this in different machines. For now let's just use the more compact solution. Thanks for your time.

[SamiNasry]

Can you please show me how does the original getline function behave for the same input in your machine ?

[SamiNasry]

This is the code of the original getline() funtion

#include <stdio.h>

int getline(char s[], int lim);

int getline(char s[], int lim)
{
	int c,i;
	for (i = 0; i < lim -1 && ((c = getchar()) != EOF) && c != '\n' ; ++i)
	{
		s[i] = c;

	}
	if (c == '\n') {
		s[i] = c;
		++i;
	}
	s[i] = '\0';
	return i;
}


int main()
{
	int lim = 10;
	char s[lim];

	
	getline(s, lim);
	printf("%s" , s);
}

[ohkimur]

@SamiNasry Here is how it behaves for me. I had to change the name, since the clang compiler doesn't like me to reuse the getline name since it's already part of the stdio lib.

[ohkimur]

@SamiNasry Can we have it without the break until we figure out what's up with that? I find it very educational to learn how to deal with the differences between different machines.

PS: I'm used to writing code in high-level programming languages, and I almost forgot about all these low-level implementation details.

[SamiNasry]

Yes I removed the break until we figure out why it acts like this. Thanks for your time.