Design... the right way.

[mike.rosset+gnus]

Hello All:


I am new to C and I have a program design question. Not only am I trying
to learn C but I'm also trying to pick up good habits. And I'm cocerned
with doing things the right and avoiding picking up bad habits.

The program I'm working on uses popen to read some pre-formatted output
from another program. The output looks like this.

line 0: persons_name
line 1: persons_last_name
line 2: .....

I then need to take each line assign it to a structure member. What
would be the simplest approach to find the member field from the line
loop count?

Michael Rosset

 

[Nick Keighley]

I am new to C and I have a program design question. Not only am I trying
to learn C but I'm also trying to pick up good habits.
good


And I'm cocerned
with doing things the right [way] and avoiding picking up bad habits.

The program I'm working on uses popen

popen() isn't actually standard C. It's standard Unix (Posix).
But your question becomes a standard C question if we assume you're
reading
from a stream (eg. opened by fopen())

to read some pre-formatted output
from another program. The output looks like this.

that is the *input* to your program looks like:

line 0: persons_name
line 1: persons_last_name
line 2: .....

I then need to take each line assign it to a structure member. What
would be the simplest approach to find the member field from the line
loop count?

I'm not sure. What are the structure members called?
When I read your question I thought persons_name
indicated a name and the actual input looked like

line 0: frodo
line 1: baggins

etc.

So you might be doing something like

void process_lines (struct Record *record)
{
char line [LARGEST_LINE];
char field [LARGEST_FIELD];
int line_count = 0;

while (read_line(line) == OK)
{
line_count++;
get_field (field, line);

switch (line_count)
{
1:
strcpy (record->person_name, field);
break;

2:
strcpy (record->persons_last_name, field);
break;

default:
fprintf (stderr, "Dire Straits!\n");
exit (EXIT_FAILURE);
}
}
}

which isn't exactly elegant...
The error handling is weak, the program is very trusting of the input
source.

I'd do get_field() more like this (a different approach to
RH's solution).

int get_field (char *field, const char *line)
{
int line_count
if (sscanf (line, "line %d: %s", &line_count, field) != 2)
return PARSE_ERROR;

return OK;
}

Could you explain your problem in more detail?


--
Nick Keighley

/*
- Note:
- I wanted to put a check in getGuiExtract(), but it's a void
method, inherited from the father of the father of the father of
the
[...]
the father of the father of the father of the father of the
father ...

So I can't modify its interface.
*/
[cry for help found in source code]

 

[Bartc]

Hello All:


I am new to C and I have a program design question. Not only am I
trying to learn C but I'm also trying to pick up good habits. And I'm
cocerned with doing things the right and avoiding picking up bad
habits.

The program I'm working on uses popen to read some pre-formatted
output from another program. The output looks like this.

line 0: persons_name
line 1: persons_last_name
line 2: .....

This is ambiguous. Is the line 0:, line 1: etc actually part of the output?
(If it is, it is unnecessary unless there could be other output
interspersed.)

Also, in place of 'persons_name' etc, you presumably have the actual
person's name?

In this case you don't know the name of the field involved, but have to
deduce it from the line number?

How do you make sure the member fields of your destination struct agree in
number and order with those from the other program?

More reliable might be:

persons_name Bart
persons_last_name Simpson
.....

This also makes it easier to have more than one person's record in a file.

In the C code, you would need a table (or some logic if there are few
fields) to map the member name to the offset of the struct involved.

I then need to take each line assign it to a structure member. What
would be the simplest approach to find the member field from the line
loop count?

If working from a line number, and you're happy with the file matching your
struct exactly, in the same way you could use a table in this case storing
the offsets of the N'th struct field.

Try submitting some code including the layout of your struct. Then we can
see where to go from there.

 

[mike.rosset+gnus]

Thanks everyone for responding. It turns out I should have just posted
some code. Sorry for wasting anyone's time. The data format looks more like this

Frodo
Baggins
Hobbit

And I added "line 0:" trying to explain I know the person's name is
always on line 0.

Basically the file I really need to parse is a bash script. tesh.sh
parses and displays the fields in a known order. I dont have
the skills to do it in C yet.

I decided it was easier to post code. Feel free to criticize it all
you want. Just keep in mind I've only been learning C the last 2 weeks.
And this whole program is a make work project.

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

#define MAX_FIELDS 19

/* package var enumerator */
enum pkg_vars {
pkgname,
pkgver
};

/* Main */
int main()
{
int lines = 0;
char *package[MAX_FIELDS][PATH_MAX];

/* popen? *waves hand* yes its fopen, continue on your way.*/
FILE *input = popen("./test.sh", "r");
char line[PATH_MAX + 1];

if(input == NULL) {
error("Error: running parser");
pclose(input);
exit(EXIT_FAILURE);
}

while(fgets(line, PATH_MAX, input)) {

line[strlen(line) - 1 ] = '\0';
if(lines < MAX_FIELDS) {
strcpy(package[lines], line);
}
lines++;
}

printf("name = %s\n", package[pkgname]);
printf("pkgver = %s\n", package[pkgver]);
printf("total = %d", lines);
pclose(input);
return 0;
}

strcpy is giving me a warning I still cant figure out.
passing argument 1 of ‘strcpy’ from incompatible pointer type

Basicaly because I know what line each field is on I'm using an
enumertator to access the array. Or should I use struct and switch? that
was my orginal plan. Package will be used heavly in the rest
of the program I'm thinking it's easier to access the package data with
pkg.pkgname . And eventually the program will need to access more then
one just one package data. possible an array of struct packages?.

Mike Rosset

 

[Bartc]

Thanks everyone for responding. It turns out I should have just posted
some code. Sorry for wasting anyone's time. The data format looks more
like this

Frodo
Baggins
Hobbit

I vaguely remember Bilbo Baggins, so are these first names or last names?

And I added "line 0:" trying to explain I know the person's name is
always on line 0.

Basically the file I really need to parse is a bash script. tesh.sh
parses and displays the fields in a known order. I dont have
the skills to do it in C yet.

I decided it was easier to post code. Feel free to criticize it all
you want. Just keep in mind I've only been learning C the last 2 weeks.
And this whole program is a make work project.

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

#define MAX_FIELDS 19

You might need PATH_MAX too, unless it's something that already exists on
your system.

* package var enumerator */
enum pkg_vars {
pkgname,
pkgver
};

int main()
{
int lines = 0;
char *package[MAX_FIELDS][PATH_MAX];

You don't need the * here, as you are statically declaring MAX_FIELD arrays
each of PATH_MAX characters.

/* popen? *waves hand* yes its fopen, continue on your way.*/
FILE *input = popen("./test.sh", "r");

I'm not familar with popen, from the docs it doesn't seem appropriate. Try
fopen().

char line[PATH_MAX + 1];

if(input == NULL) {
error("Error: running parser");

error() is not declared in this code.

pclose(input);
exit(EXIT_FAILURE);
}

while(fgets(line, PATH_MAX, input)) {

line[strlen(line) - 1 ] = '\0';
if(lines < MAX_FIELDS) {
strcpy(package[lines], line);
}
lines++;
}

printf("name = %s\n", package[pkgname]);
printf("pkgver = %s\n", package[pkgver]);
printf("total = %d", lines);
pclose(input);
return 0;
}

strcpy is giving me a warning I still cant figure out.
passing argument 1 of 'strcpy' from incompatible pointer type

Basicaly because I know what line each field is on I'm using an
enumertator to access the array. Or should I use struct and switch? that
was my orginal plan. Package will be used heavly in the rest
of the program I'm thinking it's easier to access the package data with
pkg.pkgname . And eventually the program will need to access more then
one just one package data. possible an array of struct packages?.

If all the fields are the same type then an array would easier. (Even when
they are strings of differing length, you can use an array of char*, but
then you must allocate each string.). And arrays of arrays are just as
possible as an array of structs.

For readability, pkg[pkgname] is not that different from pkg.pkgname.

 

[mike.rosset+gnus]

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

#define MAX_FIELDS 19
#define PATH_MAX 10

enum pkg_vars {
pkgname,
pkgver
};

int main(void)
/*
** non prototype declarations
** is an obsolescent feature of the language
*/
{
int lines = 0;
char package[MAX_FIELDS][PATH_MAX];
/*
** A two dimensional array of char
*/
FILE *input = fopen("./test.sh", "r");
char line[PATH_MAX + 1];

if(input == NULL) {
perror("Error: running parser");
/*
** Nothing to fclose here
*/
exit(EXIT_FAILURE);
}
while(fgets(line, PATH_MAX, input)) {
line[strlen(line) - 1 ] = '\0';
if(lines < MAX_FIELDS) {
strcpy(package[lines], line);
}
lines++;
}
printf("name = %s\n", package[pkgname]);
printf("pkgver = %s\n", package[pkgver]);
printf("total = %d\n", lines);
/*
** A portable text stream, ends in a newline character.
*/
fclose(input);
return 0;
}

Pete, thanks for your response. Thank you for for your suggestions, all
of them make sense. I have a couple of questionsi. When you say "non
prototype declarations" you mean I should declare void as a parmater, it
not to be assumed.

Also I'm not sure what you mean by a portable stream should end in a
newline. Does that mean I should'nt remove it from line or I should copy
the line then remove it?

Mike Rosset

 

[mike.rosset+gnus]

This is a better way to end main:

printf("total = %d\n", lines);
fclose(input);
return 0;
}

than this is:

printf("total = %d", lines);
fclose(input);
return 0;
}

Ah the light bulb want off. I never thought of the ouput from main as a
stream. But when you put it like that it makes perfect sense.

Is there a puts function but with formatting?


Mike

 

[Keith Thompson]

Ah the light bulb want off. I never thought of the ouput from main as a
stream. But when you put it like that it makes perfect sense.

Is there a puts function but with formatting?

Um, there's printf.

If by "a puts function" you mean a function that implicitly appends a
new-line, then no, there's no such standard function. But there's
really no need for one; it's easy enough to put a "\n" at the end of
printf's format string.

 

[David Thompson]

I vaguely remember Bilbo Baggins, so are these first names or last names?

<OT highly summarized> Frodo is indeed (fictionally) the first name of
Bilbo's honorary nephew and heir, who thus becomes the 'ring-bearer'
to take the One Ring to Mount Doom to be destroyed, through (nearly
all of) the _Lord of the Rings_ trilogy. I can imagine how you might
have ignored the books -- not forgive, just imagine <G> -- but totally

I'm not familar with popen, from the docs it doesn't seem appropriate. Try
fopen().

OP previously said he wants this program to parse _the output of
running a script namely test.sh_ so it's very appropriate. But OT.
(But the issues of parsing and storing are mostly the same whether the
input comes from a running script or a plain file, and hence ontopic.
The only obvious exception would be if you wanted to rewind or
otherwise reposition in the input, which fails for a pipe; but then in
Standard C it may fail even for a 'real' file -- if any such.)

- formerly david.thompson1 || achar(64) || worldnet.att.net