mersenneforum.org

mersenneforum.org (https://www.mersenneforum.org/index.php)
-   Programming (https://www.mersenneforum.org/forumdisplay.php?f=29)
-   -   C or C++? (https://www.mersenneforum.org/showthread.php?t=16448)

Dubslow 2012-02-26 23:03

Blasted editing time limit.

[code]char* ReadLine(FILE * stream) {
/* I don't actually have any idea what the argument prototype means, other than that "stdin" and "stdout" are legal ("stderr" as well?).
* Also, I don't think there's a way for me to use fgets() without being able to catch a \n, so I've hand coded it with getc().
* This function returns a pointer to the array where the line is stored. The array is guaranteed to be exactly as long as the string.
* This requires the inclusion of <stdio.h> and <stdlib.h>.
*/

long int size=20; // This determines the minumum mem required; if it's too low though, then you have to loop more often to increase dynamically.
// (Use a long in case you have a >4GiB string :] )
int i=1;
char* input = (char*)malloc( size*sizeof(char));
if( input == NULL ) {
printf("Error, out of memory!\n"); /* I think I should be using some sort of stderr here, but I don't actually know too much about I/O, */
return NULL; /* I'm just trying to practice with strings. */
}
if( ((input[0]=[U]getc(stream)[/U]) == EOF) || (input[0]=='\n') )
return NULL;

while(1) { // This loop is broken when '\n' is read, and then we return out of the function (or mem error returns NULL).

for(/* The first time through i==1 */ ; i<(size - 1); i++) {
if( (input[i] = [U]getc(stream)[/U]) == '\n' || ( input[i] == EOF ) ) {
input[i] = 0;
input = (char*)realloc( input, (i+1)*sizeof(char) );
if( input == NULL ) {
printf("Error, out of memory!\n");
return NULL;
}
return input;
}
}
if( ((input[i]=[U]getc(stream)[/U]) != '\n') && (input[i] != EOF) ) {
input = (char*)realloc( input, [U](size *= 2)[/U]*sizeof(char) ); // Follow bsquared's usual practice.
if( input == NULL ) {
printf("Error, out of memory!\n");
return NULL;
}
i++; // We're at i==size, continue reading at i==(size+1)
} // skip to end while, continue reading in input up above
else { input[i] = 0; return input; }
} // end while
} // end function[/code]
Should I be using stream or *stream?

chalsall 2012-02-26 23:17

[QUOTE=Dubslow;290983]Should I be using stream or *stream?[/QUOTE]

"man fopen"....

Dubslow 2012-02-27 01:38

Got distracted by a trip to BDubs...

This is the same thing fixed to conform to more detailed specs, i.e. a \n gives a \0, not NULL.
[code]char* ReadLine(FILE * stream) {
/* I don't actually have any idea what the argument prototype means, other than that "stdin" and "stdout" are legal ("stderr" as well?).
* Also, I don't think there's a way for me to use fgets() without being able to catch a \n, so I've hand coded it with getc().
* This function returns a pointer to the array where the line is stored. The array is guaranteed to be exactly as long as the string.
* This requires the inclusion of <stdio.h> and <stdlib.h>.
*/

long int size=20; // This determines the minumum mem required; if it's too low though, then you have to loop more often to increase dynamically.
// (Use a long in case you have a >4GiB string :] )
int i=1;
char* input = (char*)malloc( size*sizeof(char));
if( input == NULL ) {
printf("Error, out of memory!\n"); /* I think I should be using some sort of stderr here, but I don't actually know too much about I/O, */
return NULL; /* I'm just trying to practice with strings. */
}
if( ((input[0]=getc(stream)) == EOF)) {
return NULL;
} else if( input[0] == '\n' ) {
i=0;
goto exit; // Can't define functions inside functions, and the code is already down there, so may as well use it.
}

while(1) { // This loop is broken when '\n' is read, and then we return out of the function (or mem error returns NULL).

for(/* The first time through i==1 */ ; i<(size - 1); i++) {
if( (input[i] = getc(stream)) == '\n' || ( input[i] == EOF ) ) {
/* goto target */ exit: input[i] = 0; // With the proper i, we can "go here" from anywhere to return the input
input = (char*)realloc( input, (i+1)*sizeof(char) );
if( input == NULL ) {
printf("Error, out of memory!\n");
return NULL;
}
return input;
}
}
if( ((input[i]=getc(stream)) != '\n') && (input[i] != EOF) ) {
input = (char*)realloc( input, (size *= 2)*sizeof(char) ); // Follow bsquared's usual practice.
if( input == NULL ) {
printf("Error, out of memory!\n");
return NULL;
}
i++; // We're at i==size, continue reading at i==(size+1)
} // skip to end while, continue reading in input up above
else { input[i] = 0; return input; }
} // end while
} // end function[/code]
I got an idea for this based on the previous iteration; it's perhaps more questionable style, but it definitely is fewer lines of code.
[code]char* ReadLine(FILE * stream) {
/* I don't actually have any idea what the argument prototype means, other than that "stdin" and "stdout" are legal ("stderr" as well?).
* Also, I don't think there's a way for me to use fgets() without being able to catch a \n, so I've hand coded it with getc().
* This function returns a pointer to the array where the line is stored. The array is guaranteed to be exactly as long as the string.
* This requires the inclusion of <stdio.h> and <stdlib.h>.
*/

long int size=20; // This determines the minumum mem required; if it's too low though, then you have to loop more often to increase dynamically.
// (Use a long in case you have a >4GiB string :] )
int i=1;
char* input = (char*)malloc( size*sizeof(char));
if( input == NULL ) {
goto error;
}
if( ((input[0]=getc(stream)) == EOF)) {
return NULL;
} else if( input[0] == '\n' ) {
i=0;
goto exit;
}

while(1) { // This loop is broken when '\n' is read, and then we return out of the function (or mem error returns NULL).

for(/* The first time through i==1 */ ; i<(size - 1); i++) {
if( (input[i] = getc(stream)) == '\n' || ( input[i] == EOF ) )
goto exit;
}
if( ((input[i]=getc(stream)) != '\n') && (input[i] != EOF) ) {
input = (char*)realloc( input, (size *= 2)*sizeof(char) ); // Follow bsquared's usual practice.
if( input == NULL ) {
goto error;
}
i++; // We're at i==size, continue reading at i==(size+1)
} // skip to end while, continue reading in input up above
else { input[i] = 0; return input; }
} // end while

/* goto targets */
/* Can't define functions inside functions, and the code is already down here, no need to repeat it. */
/* Error comes before exit just in case. */

error: printf("Error, out of memory!\n"); /* I think I should be using some sort of stderr here, but I don't actually know too much about I/O, */
return NULL; /* I'm just trying to practice with strings. */

exit: input[i] = 0; // With the proper i, we can "go here" from anywhere to return the input
input = (char*)realloc( input, (i+1)*sizeof(char) );
if( input == NULL )
goto error;
return input;

} // end function[/code]

Dubslow 2012-02-27 02:52

Okay, since jyb seemed to be saying there was more than one bug in ReadLine, I went testing cyhper with more phrases. (I'm using the most recent version of ReadLine, the second version from the post above.) I found a phrase that breaks cypher, but not ReadLine, which as far as I can tell (as of yet) does its job great.
[code]bill@Gravemind:~/bin/c∰∂ cypher
Please enter the shift value (between -25..-1 and 1..25)
14
Using shift value of 14
Please enter the source text (empty line to quit)
Check the parameter AllowSleep=1 in the mfaktc.ini file.
Source : Check the parameter AllowSleep=1 in the mfaktc.ini file.
Processed: Qvsqy hvs dofoashsf OzzckGzssd=1 wb hvs atoyhq.wbw twzs.a*
Please enter the source text (empty line to quit)

Bye.
bill@Gravemind:~/bin/c∰∂[/code]
Where the * is, on my terminal, a square box that has a 0-0-0-2 on the inside. (That's the character I get if I do 'printf("%c", 2);' )
I don't know why extra characters are tacked on; I ran it through gdb.
[code](gdb) p code
$69 = 0x603070 "Qvsqy hvs dofoashsf OzzckGzssd=1 wb hvs atoyhq.wbw twz"
(gdb) n
88 } else if (usecret) {
(gdb)
91 c = (char) (a + ((c - a + shift) % 26));
(gdb)
93 a=0;
(gdb)
95 code[i] = c;
(gdb)
79 for ( i=0; ((c=string[i])!=0) && (i<size) ; i++) {
(gdb) p code
$70 = 0x603070 "Qvsqy hvs dofoashsf OzzckGzssd=1 wb hvs atoyhq.wbw twzs"
(gdb) n
80 if ( isULetter(c) ) {
(gdb) p c
$71 = 46 '.'
(gdb) n
82 } else if( isLLetter(c) ) {
(gdb) n
85 if( a ) {
(gdb) n
95 code[i] = c;
(gdb) p c
$72 = 46 '.'
(gdb) n
79 for ( i=0; ((c=string[i])!=0) && (i<size) ; i++) {
(gdb) p c
$73 = 46 '.'
(gdb) p code
$74 = 0x603070 "Qvsqy hvs dofoashsf OzzckGzssd=1 wb hvs atoyhq.wbw twzs.a\017\002"
(gdb) [/code]
I don't get it. Doing code[55]='.'; appears to also do code[56]='a' and code[57]=2. Any ideas? The full source of cypher.c . I've commented the apparent problem area in all caps.
[code]#include <stdio.h>
#include <stdlib.h>
#include <string.h>

/* A program to take a string, and create its Ceasar Cypher using a shift value input by the user.
* (Entering 999 or -999 uses a letter's position as its shift value instead.)
*/

char* ReadLine(FILE * stream);

int isULetter(char c) { // boolean
if ('A' <= c && c <= 'Z')
return 1;
else
return 0;
}

int isLLetter(char c) { // boolean
if ('a' <= c && c <= 'z')
return 1;
else
return 0;
}

int main(void) {
int shift, size;
int badin; // boolean
int secret = 0; // boolean
int usecret = 0;// boolean
int i; // index
char c;
char* scan = (char *)malloc( 10 * sizeof(char));
char* string;
char* code;
char a=0;
do {
printf("Please enter the shift value (between -25..-1 and 1..25)\n");
fgets( scan, 10, stdin);
badin = (sscanf(scan, "%d", &shift) != 1)
||
( !(((-25 <= shift) && (shift <= -1)) || ((1 <= shift) && (shift <= 25))) );
/* A somewhat obfuscated line of code; if shift is not in [-25,-1]U[1,25] OR sscanf didn't assign correctly, then badin is set to true. */

if (shift == 999) {
secret = 1;
badin = 0;
} else if (shift == -999) {
usecret = 1;
badin = 0;
}
if (badin)
printf("%d is not a valid shift value.\n", shift);
} while (badin);
free(scan);

if (secret || usecret)
printf("Using position shift\n");
else
printf("Using shift value of %d\n", shift);
if (shift < 0)
shift += 26;

while (1) {
printf("Please enter the source text (empty line to quit)\n");
string = ReadLine(stdin);
if ( string == NULL ) { // Either there was an error, or blank input. (Error message would be printed if there was a error.)
printf("There was an error above!\n");
return 7;
} else if( string[0] == 0 ) {
printf("Bye.\n");
return 0;
}
size = (int)strlen(string);

printf("Source : ");
puts(string);

code = (char *)malloc( size * sizeof(char));
for ( i=0; ((c=string[i])!=0) && (i<size) ; i++) {
if ( isULetter(c) ) {
a='A';
} else if( isLLetter(c) ) {
a='a';
}
if( a ) {
if (secret) {
c = (char) (a + ((c - a + i) % 26));
} else if (usecret) {
c = (char) (a + ((c - a + (26 - (i % 26))) % 26));
} else {
c = (char) (a + ((c - a + shift) % 26));
}
a=0;
}
code[i] = c;
/* THIS IS WHERE THE CODE APPEARS TO GO CRAZY. */
}

printf("Processed: ");
puts(code);
free(string);
free(code);

} // while
} // main

char* ReadLine(FILE * stream) {
/* I don't actually have any idea what the argument prototype means, other than that "stdin" and "stdout" are legal ("stderr" as well?).
* Also, I don't think there's a way for me to use fgets() without being able to catch a \n, so I've hand coded it with getc().
* This function returns a pointer to the array where the line is stored. The array is guaranteed to be exactly as long as the string.
* This requires the inclusion of <stdio.h> and <stdlib.h>.
*/

long int size=20; // This determines the minumum mem required; if it's too low though, then you have to loop more often to increase dynamically.
// (Use a long in case you have a >4GiB string :] )
int i=1;
char* input = (char*)malloc( size*sizeof(char));
if( input == NULL ) {
goto error;
}
if( ((input[0]=getc(stream)) == EOF)) {
return NULL;
} else if( input[0] == '\n' ) {
i=0;
goto exit;
}

while(1) { // This loop is broken when '\n' is read, and then we return out of the function (or mem error returns NULL).

for(/* The first time through i==1 */ ; i<(size - 1); i++) {
if( (input[i] = getc(stream)) == '\n' || ( input[i] == EOF ) )
goto exit;
}
if( ((input[i]=getc(stream)) != '\n') && (input[i] != EOF) ) {
input = (char*)realloc( input, (size *= 2)*sizeof(char) ); // Follow bsquared's usual practice.
if( input == NULL ) {
goto error;
}
i++; // We're at i==size, continue reading at i==(size+1)
} // skip to end while, continue reading in input up above
else { input[i] = 0; return input; }
} // end while

/* goto targets */
/* Can't define functions inside functions, and the code is already down here, no need to repeat it. */
/* Error comes before exit just in case. */

error: printf("Error, out of memory!\n"); /* I think I should be using some sort of stderr here, but I don't actually know too much about I/O, */
return NULL; /* I'm just trying to practice with strings. */

exit: input[i] = 0; // With the proper i, we can "go here" from anywhere to return the input
input = (char*)realloc( input, (i+1)*sizeof(char) );
if( input == NULL )
goto error;
return input;

} // end function[/code]

jyb 2012-02-27 03:25

[QUOTE=Dubslow;290993]Okay, since jyb seemed to be saying there was more than one bug in ReadLine, I went testing cyhper with more phrases. (I'm using the most recent version of ReadLine, the second version from the post above.) I found a phrase that breaks cypher, but not ReadLine, which as far as I can tell (as of yet) does its job great.

[snip]

I don't get it. Doing code[55]='.'; appears to also do code[56]='a' and code[57]=2. Any ideas? The full source of cypher.c . I've commented the apparent problem area in all caps.
[/QUOTE]

After the call to ReadLine, how long is the string (as returned by strlen)? And therefore how much space did ReadLine allocate for it?

When you allocate space for code, how much are you giving it? And if you give it what it needs, are you actually filling in everything you need to?

jyb 2012-02-27 03:30

[QUOTE=chalsall;290984]
[QUOTE=Dubslow;290983]
Should I be using stream or *stream?
[/QUOTE]
"man fopen"....[/QUOTE]

In case following chalsall's good advice didn't fully clarify matters for you, think of it this way: what type does stream have in your ReadLine function? What type does getc expect? Do the answers to those questions tell you what you need to know?

Dubslow 2012-02-27 03:36

[QUOTE=jyb;290995]After the call to ReadLine, how long is the string (as returned by strlen)? [/quote]56[QUOTE=jyb;290995]And therefore how much space did ReadLine allocate for it? [/quote]57[QUOTE=jyb;290995]
When you allocate space for code, how much are you giving it? [/quote]56[QUOTE=jyb;290995] And if you give it what it needs, are you actually filling in everything you need to?[/QUOTE]I don't terminate code with a null, since this prog only prints it to stdout. The string has 56 characters, so code[55]='.' and then print it. There is no code[>=56], because I only allocated it to be 56.
[code]Starting program: /home/bill/bin/c/cypher
Please enter the shift value (between -25..-1 and 1..25)
14
Using shift value of 14
Please enter the source text (empty line to quit)
Check the parameter AllowSleep=1 in the mfaktc.ini file.
Source : Check the parameter AllowSleep=1 in the mfaktc.ini file.

Breakpoint 1, main () at cypher.c:78
78 code = (char *)malloc( size * sizeof(char));
(gdb) p size
$75 = 56
(gdb) p string[55]
$76 = 46 '.'
(gdb) p string [56]
$77 = 0 '\000'
(gdb) p string [57]
$78 = 0 '\000'
(gdb) p string
$79 = 0x603010 "Check the parameter AllowSleep=1 in the mfaktc.ini file."
(gdb) [/code]
Edit: Does puts() require the null terminator? If it does, why did the other 5 test cases work fine then?
[code]bill@Gravemind:~/bin/c∰∂ cypher
Please enter the shift value (between -25..-1 and 1..25)
999
Using position shift
Please enter the source text (empty line to quit)
This is testing the final period case.
Source : This is testing the final period case.
Processed: Tikv ny bnceuau jyw zdjxj pftlsi jibo.
Please enter the source text (empty line to quit)

Bye.[/code]

Dubslow 2012-02-27 03:42

[QUOTE=jyb;290996]In case following chalsall's good advice didn't fully clarify matters for you, think of it this way: what type does stream have in your ReadLine function? What type does getc expect? Do the answers to those questions tell you what you need to know?[/QUOTE]
I have absolutely no idea about streams or anything. That's one of the later chapters in the reference I've been using.
[url]http://publications.gbdirect.co.uk/c_book/[/url]
After I've got this string thing sorted out the next thing was going to be the Ch. 5 exercises, and then Ch. 6.

jyb 2012-02-27 04:01

[QUOTE=Dubslow;290997]I don't terminate code with a null, since this prog only prints it to stdout. The string has 56 characters, so code[55]='.' and then print it. There is no code[>=56], because I only allocated it to be 56.
[/QUOTE]
Yes, and this is your problem.

[QUOTE=Dubslow;290997]
Edit: Does puts() require the null terminator?
[/QUOTE]
Yes it does. There are two ways I can explain why. One is sort of a formal "this is the way it is in C" kind of thing, and you'll shrug your shoulders and say okay. The other will probably make you say "Oh yeah, I could have figured that out."

First one: in C, a "string" isn't just any old array of characters. It is quite specifically an array of characters up to a terminating null character. Functions like puts (and printf with a %s conversion) which expect a string must receive an argument of this form.

Second one: How do you think puts knows where the string ends? When you pass a string to a function, all you're really giving it is a pointer to the first character. How can it tell how far to look? Lest you think that maybe it knows based on the space allocated, you need to remember two things: 1) there is no requirement that the part you care about (i.e. everything up to the null character) actually fills the allocated space; I can declare an array 80 long, then put in just the characters 'h', 'i' and '\0' and puts will think this is a string of length 2. And 2) you already discovered earlier in this discussion that there's no way to ask how big a "chunk of memory" is. So puts (and any other function that handles strings) MUST have the terminating null character so it knows where the string ends.

[QUOTE=Dubslow;290997]
If it does, why did the other 5 test cases work fine then?
[/QUOTE]

In essence, you got lucky. For whatever reason, the next byte of memory after the characters you filled happened to be 0, so it worked by coincidence. Alternatively, every byte up the next 0 byte happened to represent a non-printing character, so it didn't screw up the output and it appeared to work.

BTW, that last phenomenon happens in C *all the time*. You get lucky and things appear to work so you think you're bug-free. Then you try running the same old code in a new way and it blows up. This is why really good C programming requires a certain level of rigor and understanding. It's not enough to be willing to experiment a lot (though that can help); you really have to have a good understanding of what's really going on. Then you can be more confident that things work because you've done them right, not because you're getting lucky.

chalsall 2012-02-27 04:09

[QUOTE=Dubslow;290997]Edit: Does puts() require the null terminator?[/QUOTE]

Well, think about it...

The only parameter to the puts function is a char *.

Without depending on the string being NULL terminated, how would it know when to stop "putting"?

[QUOTE=Dubslow;290997]If it does, why did the other 5 test cases work fine then?[/QUOTE]

You were lucky -- the bytes following happened to be zero. But this might not always be the case.

Dubslow... this is meant to be constructive criticism: from my perspective you guess too much on how a function, structure or methodology works. I know you're only just beginning to learn C, and your efforts are to be commended.

However, C is one of those languages where you have to understand [B][I][U]exactly[/U][/I][/B] how things work or you're going to find yourself in big trouble really fast. Just because something runs in your environment does not necessarily mean the code is correct -- it could crash in another. Or, perhaps even worse, manifest the "once a month bug" -- difficult to reproduce; almost impossible to debug.

You need to learn to use the "man" pages more often. Rather than asking questions here, figure it out for yourself from the definitions for the functions you're using.

jyb 2012-02-27 04:19

[QUOTE=Dubslow;291000]I have absolutely no idea about streams or anything. That's one of the later chapters in the reference I've been using.
[url]http://publications.gbdirect.co.uk/c_book/[/url]
After I've got this string thing sorted out the next thing was going to be the Ch. 5 exercises, and then Ch. 6.[/QUOTE]

Perhaps, but you at least know enough to be able to look up what type getc expects and look at how you've declared the "stream" argument in your function. Knowing that they're the same is all that you need in order to answer your question from before.

As for "this string thing," I might recommend that you spend a little more time refining your ReadLine function. It may be working for you now, but when I said before that it had "some structural problems" I wasn't merely talking about correctness. The function as you've written it is more complicated than it needs to be, in a way which makes it unintuitive for the reader (e.g. the nested loops). I'm confident from what I've seen that you can come up with a version that is simpler, shorter and clearer, all without needing to use any gotos. :wink:

Now of course this might just be something you wanted to get working and you don't want to be bothered with making it perfect, in which case I totally understand and will leave you alone. But in case you viewed it more as an opportunity to learn, then there is more that even a small example such as this can teach you, and you may want to take advantage of it (and of the people on this forum who are willing to help you).


All times are UTC. The time now is 21:46.

Powered by vBulletin® Version 3.8.11
Copyright ©2000 - 2021, Jelsoft Enterprises Ltd.