Ugly C code
-
Following to my question yesterday, is there anything that can be done to clean up the c code below which works fine but looks ugly especially the strcat's? The program parses a file containing a list of files and then runs a command on each. I can do a similar thing in perl like this a couple of lines but in C its looks ugly and I think its because I am a newbie...any comments will be appreciated.... thanks in advance for everyones help In perl: #!/usr/bin/perl while(<>){ chomp; $cmd="qsub -P 9999 -wd -e $_\.error -b y /usr/local/projects/bin/program -i $_ -type p -terms -lookup -o $_\.qsub"; print "$cmd\n"; } in C: #include #include #include #include #include #include int main(int argc, char *argv[]) { char * cwd = getcwd(0,0); char pro[10000]; char str[1000]; char* nl; if (argc < 3) { printf("\n\nPlease supply a file containing a list of single file names follow by project code\n"); exit(5); } FILE *f; f = fopen(argv[1], "r"); if(!f) { printf("Couldn't open file.txtn"); return 1; } while(fgets(str, 1000, f)) { nl = strrchr(str, '\r'); if (nl) *nl = '\0'; nl = strrchr(str, '\n'); if (nl) *nl = '\0'; strcpy(pro, "qsub -P "); strcat(pro, argv[2]); strcat(pro, " -wd -e "); strcat(pro, cwd); strcat(pro, "/"); strcat(pro, str); strcat(pro, ".error -b y /usr/local/projects/bin/program -i "); strcat(pro, cwd); strcat(pro, "/"); strcat(pro, str); strcat(pro, " -type p -terms -lookup -o "); strcat(pro, str); strcat(pro, ".qsub"); printf (pro); system(pro); } }
-
Following to my question yesterday, is there anything that can be done to clean up the c code below which works fine but looks ugly especially the strcat's? The program parses a file containing a list of files and then runs a command on each. I can do a similar thing in perl like this a couple of lines but in C its looks ugly and I think its because I am a newbie...any comments will be appreciated.... thanks in advance for everyones help In perl: #!/usr/bin/perl while(<>){ chomp; $cmd="qsub -P 9999 -wd -e $_\.error -b y /usr/local/projects/bin/program -i $_ -type p -terms -lookup -o $_\.qsub"; print "$cmd\n"; } in C: #include #include #include #include #include #include int main(int argc, char *argv[]) { char * cwd = getcwd(0,0); char pro[10000]; char str[1000]; char* nl; if (argc < 3) { printf("\n\nPlease supply a file containing a list of single file names follow by project code\n"); exit(5); } FILE *f; f = fopen(argv[1], "r"); if(!f) { printf("Couldn't open file.txtn"); return 1; } while(fgets(str, 1000, f)) { nl = strrchr(str, '\r'); if (nl) *nl = '\0'; nl = strrchr(str, '\n'); if (nl) *nl = '\0'; strcpy(pro, "qsub -P "); strcat(pro, argv[2]); strcat(pro, " -wd -e "); strcat(pro, cwd); strcat(pro, "/"); strcat(pro, str); strcat(pro, ".error -b y /usr/local/projects/bin/program -i "); strcat(pro, cwd); strcat(pro, "/"); strcat(pro, str); strcat(pro, " -type p -terms -lookup -o "); strcat(pro, str); strcat(pro, ".qsub"); printf (pro); system(pro); } }
Hi, why don't you use
sprintf()
to compute the entire string? it works similar to printf() but outputs to a char buffer. FYI: More modern languages support string objects and do simple concatenation, much like ancient Basic:string c = a + b;
:)Luc Pattyn [Forum Guidelines] [My Articles]
- before you ask a question here, search CodeProject, then Google - the quality and detail of your question reflects on the effectiveness of the help you are likely to get - use the code block button (PRE tags) to preserve formatting when showing multi-line code snippets
-
Following to my question yesterday, is there anything that can be done to clean up the c code below which works fine but looks ugly especially the strcat's? The program parses a file containing a list of files and then runs a command on each. I can do a similar thing in perl like this a couple of lines but in C its looks ugly and I think its because I am a newbie...any comments will be appreciated.... thanks in advance for everyones help In perl: #!/usr/bin/perl while(<>){ chomp; $cmd="qsub -P 9999 -wd -e $_\.error -b y /usr/local/projects/bin/program -i $_ -type p -terms -lookup -o $_\.qsub"; print "$cmd\n"; } in C: #include #include #include #include #include #include int main(int argc, char *argv[]) { char * cwd = getcwd(0,0); char pro[10000]; char str[1000]; char* nl; if (argc < 3) { printf("\n\nPlease supply a file containing a list of single file names follow by project code\n"); exit(5); } FILE *f; f = fopen(argv[1], "r"); if(!f) { printf("Couldn't open file.txtn"); return 1; } while(fgets(str, 1000, f)) { nl = strrchr(str, '\r'); if (nl) *nl = '\0'; nl = strrchr(str, '\n'); if (nl) *nl = '\0'; strcpy(pro, "qsub -P "); strcat(pro, argv[2]); strcat(pro, " -wd -e "); strcat(pro, cwd); strcat(pro, "/"); strcat(pro, str); strcat(pro, ".error -b y /usr/local/projects/bin/program -i "); strcat(pro, cwd); strcat(pro, "/"); strcat(pro, str); strcat(pro, " -type p -terms -lookup -o "); strcat(pro, str); strcat(pro, ".qsub"); printf (pro); system(pro); } }
You should really use sprintf() instead of strcpy and strcat, it will look something like this: sprintf(pro, "%s%s%s%s%c%s%s%s%c%s%s%s%s", "qsub -P ", argv[2], " -wd -e ", cwd, '/', str, ".error -b y /usr/local/projects/bin/program -i ", cwd, '/', str, " -type p -terms -lookup -o ", str, ".qsub"); after this call pro[] buffer contains formatted string :-) And few remarks about your code safety: be careful with creating huge local buffers on the stuck - pro[] buffer in this case. It may happen that you get stuck overflow (application crush) It is better to create such buffers dynamically then you can check if allocation succeeded or just create static buffer use sizeof(str) in fgets(str, sizeof(str), f) to avoid problems with writing outsie str[] memory area when str[] buffer size will be changed to smaller one
-
You should really use sprintf() instead of strcpy and strcat, it will look something like this: sprintf(pro, "%s%s%s%s%c%s%s%s%c%s%s%s%s", "qsub -P ", argv[2], " -wd -e ", cwd, '/', str, ".error -b y /usr/local/projects/bin/program -i ", cwd, '/', str, " -type p -terms -lookup -o ", str, ".qsub"); after this call pro[] buffer contains formatted string :-) And few remarks about your code safety: be careful with creating huge local buffers on the stuck - pro[] buffer in this case. It may happen that you get stuck overflow (application crush) It is better to create such buffers dynamically then you can check if allocation succeeded or just create static buffer use sizeof(str) in fgets(str, sizeof(str), f) to avoid problems with writing outsie str[] memory area when str[] buffer size will be changed to smaller one
-
One little (possible) improvement to the original answer - the literal strings can be embedded in the sprintf format string, as shown below. It's a bit more like variable interpolation in strings, a la Perl.
sprintf(pro, "qsub -P %s -wd -e %s/%s.error -b y /usr/local/projects/bin/program -i %s/%s -type p -terms -lookup -o %s.qsub",
argv[2],
cwd,
str,
cwd,
str,
str);Java, Basic, who cares - it's all a bunch of tree-hugging hippy cr*p
-
One little (possible) improvement to the original answer - the literal strings can be embedded in the sprintf format string, as shown below. It's a bit more like variable interpolation in strings, a la Perl.
sprintf(pro, "qsub -P %s -wd -e %s/%s.error -b y /usr/local/projects/bin/program -i %s/%s -type p -terms -lookup -o %s.qsub",
argv[2],
cwd,
str,
cwd,
str,
str);Java, Basic, who cares - it's all a bunch of tree-hugging hippy cr*p