«

Hi, I am writing a shell for a class and I have to write some builtins
such as alias and cd. I am having some trouble with alias. I anyone
could give me some ideas about how to do it I would greatly appreciate
it. I have some code that I began but I am having some segmentation
fault. My code is:

/* the parser function parses a single pointer based on the second
argument. It returns a double pointer*/

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

#include "parser.h"

struct aliases
{
char *original;
char *replacement;
};

struct aliases *list=NULL;


int alias(char *command)
{
printf("in alias\n");
char **temp;
char **temp2;
char *s = strstr(command," = ");
if (s)
{
printf("alias: =:not found\n");
return 0;
}
temp = parser(command,'=');
temp2 = parser(temp[0],' ');
printf("about to set original\n");
char *ori;
ori = (char *)malloc(sizeof(char)*256);
char *rep;
rep = (char *)malloc(sizeof(char)*256);
strcpy(ori,temp2[1]);
strcpy(rep,temp[1]);
printf("ori:%s\n",ori);
printf("rep:%s\n",rep);
(*list).original=ori;
printf("original:%s\n",(*list).original);
(*list).replacement = rep;
printf("replacement:%s\n",(*list).replacement);
return 0;
}

the segmentation fault occurs at "(*list).original=ori". Basically so
far if I enter "alias ls="ls -l" " it should print "original:ls" and
"replacement:"ls -l" ".

Natalie

lnatz wrote:

> char *ori;
> ori = (char *)malloc(sizeof(char)*256);

How many times must people here have to say "do not cast the return
value of malloc"? Does anyone read the archive before they post? The
sizeof(char) is superfluous as well, sizeof(char) is by definition, 1.

char* ori = malloc(256);

One nice, clear, line.

> char *rep;
> rep = (char *)malloc(sizeof(char)*256);
> strcpy(ori,temp2[1]);
> strcpy(rep,temp[1]);
> printf("ori:%s\n",ori);
> printf("rep:%s\n",rep);
> (*list).original=ori;

list hasn't been assign a value. Why write (*list) rather than list->?

--
Ian Collins.

"lnatz" <> wrote in message
> Hi, I am writing a shell for a class and I have to write some builtins
> such as alias and cd. I am having some trouble with alias. I anyone
> could give me some ideas about how to do it I would greatly appreciate
> it. I have some code that I began but I am having some segmentation
> fault. My code is:
>
Are you a lisper for Harvard Univeristy, by any chance?
This is an unusal newbie post in that the task sems quite advanced.
>
> /* the parser function parses a single pointer based on the second
> argument. It returns a double pointer*/
>
That's better than no comment. However it only tell me broadly what the
parser is meant to achieve. Presumably the first p;ointer is a char * giving
the sxript, but what are the two pointers it returns?

> #include <stdio.h>
> #include <stdlib.h>
> #include <unistd.h>
> #include <string.h>
>
> #include "parser.h"
>
> struct aliases
> {
> char *original;
> char *replacement;
> };
>
Comment every structure meber with a brief comment..

> struct aliases *list=NULL;
>
And especially every global variable.

>
Then comment functions like this
/*
alias - make an alias of a variable
Params: command - a command string of the form "lvalue = lvalue"
Returns: 0.
*/
> int alias(char *command)
> {
> printf("in alias\n");
> char **temp;
> char **temp2;
> char *s = strstr(command," = ");
> if (s)
> {
> printf("alias: =:not found\n");
> return 0;
> }
>
Should be if ! s, surely. strstr will return NULL if no =.
>
> temp = parser(command,'=');
> temp2 = parser(temp[0],' ');
> printf("about to set original\n");
> char *ori;
> ori = (char *)malloc(sizeof(char)*256);
> char *rep;
> rep = (char *)malloc(sizeof(char)*256);
> strcpy(ori,temp2[1]);
> strcpy(rep,temp[1]);
>
These seem fine, as long as parser never returns anything silly.
>
> printf("ori:%s\n",ori);
> printf("rep:%s\n",rep);
> (*list).original=ori;
> printf("original:%s\n",(*list).original);
> (*list).replacement = rep;
> printf("replacement:%s\n",(*list).replacement);
> return 0;
> }
>
> the segmentation fault occurs at "(*list).original=ori". Basically so
> far if I enter "alias ls="ls -l" " it should print "original:ls" and
> "replacement:"ls -l" ".
>
> Natalie
>

You need to learn your tools as well as more fundamental programming
concepts. C programs constantly crash during development due to unitialised
or miscalculated pointers. It is important not be be firhstened of a
segmentation fault. In this case, list was probably null. That is the sang
with global variables; when called in context the function might work.
Probably you need anothe global called Naliases to keep track of the numebr
of aliases in your system. Then call realloc(aliases, (Naliases + 1) *
sizeof(struct alias)) to grow the list.

Malcolm McLean wrote:
> "lnatz" <> wrote in message
>>
>>struct aliases
>>{
>> char *original;
>> char *replacement;
>>};
>>
>
> Comment every structure meber with a brief comment..
>
Why? Just give them meaningful names, so their use is obvious in context.

--
Ian Collins.

"Ian Collins" <ian-> wrote in message
> Malcolm McLean wrote:
>
>> Comment every structure meber with a brief comment..
>>
> Why? Just give them meaningful names, so their use is obvious in context.
>
I know.
typedef struct{
double x;
double y;
double z;
} Point;

Doesn't need any comment.

OTOH

typedef struct
{
char name[64]; /* firstname, lastname eg Fred Bloggs */
float salary; /* dollars per month */
} EMPLOYEE;

is handy. A name is obviously a human -readable character string identifying
an individual, but it could be Bloggs, Bloggs F, Mr Fred Bloggs, Mr Fred
Bloggs FRS or whatever. Similalry salary might be monthly or yearly, in some
countries may be denominated in either local currency or dollars.

Malcolm McLean wrote:
> "Ian Collins" <ian-> wrote in message
>
>>Malcolm McLean wrote:
>>
>>
>>>Comment every structure meber with a brief comment..
>>>
>>
>>Why? Just give them meaningful names, so their use is obvious in context.
>>
>
> I know.
> typedef struct{
> double x;
> double y;
> double z;
> } Point;
>
> Doesn't need any comment.
>
> OTOH
>
> typedef struct
> {
> char name[64]; /* firstname, lastname eg Fred Bloggs */
> float salary; /* dollars per month */
> } EMPLOYEE;
>
typedef struct
{
char firstnameLastname[64];
float dollarsPerMonth;
} EMPLOYEE;

Verbose, but at least you don't have to look at the header to see what
the members represent.

--
Ian Collins.

Ian Collins wrote:

> typedef struct
> {
> char firstnameLastname[64];
> float dollarsPerMonth;
> } EMPLOYEE;
>
> Verbose, but at least you don't have to look at the header to see what
> the members represent.

However that's probably a good idea in the long run. Personally I abhore
camel-back naming and/or overly long names, but that's just MO.

Christopher Layne wrote:
> Ian Collins wrote:
>
>
>>typedef struct
>>{
>> char firstnameLastname[64];
>> float dollarsPerMonth;
>>} EMPLOYEE;
>>
>>Verbose, but at least you don't have to look at the header to see what
>>the members represent.
>
>
> However that's probably a good idea in the long run. Personally I abhore
> camel-back naming and/or overly long names, but that's just MO.

I dislike overly long names, so the above structure would still be a
smell to me. If the currency is important, make it a member. If you
want the first and last names, make them members. So I would end up
with something like

typedef enum { Dollars, Peanuts } Currency;
enum { NameLength = 32 };

typedef struct
{
char firstname[NameLength];
char lastname[NameLength];
Currency currency;
float monthlySalary;
} Employee;

Clear, unambiguous and not overly long.

--
Ian Collins.

"Ian Collins" <ian-> wrote in message [64];
>
>>> float dollarsPerMonth;
>>>} EMPLOYEE;
>>>
>>>Verbose, but at least you don't have to look at the header to see what
>>>the members represent.
>>
>>
>> However that's probably a good idea in the long run. Personally I abhore
>> camel-back naming and/or overly long names, but that's just MO.
>
> I dislike overly long names, so the above structure would still be a
> smell to me. If the currency is important, make it a member. If you
> want the first and last names, make them members. So I would end up
> with something like
>
> typedef enum { Dollars, Peanuts } Currency;
> enum { Nnews:...> Christopher Layne
> wrote:
>> Ian Collins wrote:
>>
>>
>>>typedef struct
>>>{
>>> char firstnameLastnameameLength = 32 };
>
> typedef struct
> {
> char firstname[NameLength];
> char lastname[NameLength];
> Currency currency;
> float monthlySalary;
> } Employee;
>
> Clear, unambiguous and not overly long.
>
But lots of snags.

Firstly, it is probably better to have names stored as familair name,
surname, other intials. I'm sure that people who work with this type of data
have long since figured out standards. But you are not necessarily in
control. If other parts of the application require the name as a single
ASCIIZ string, then it is just going to be a nuisance to have endless
conversion routines.

Secodly, names ought to be short.

By accesseing everythign through an

Employee * employee;

you have already added a layer of complexity. The rule of two states that a
human-readable name can have two elements. So you've used your two with

employee->name;

employee->first_name is a parse job.

consider

strcpy(new_employee->first_name,
candiate_employees[result->success].first_name);

virtually unreadable, but all we are doing is copying over a field.

You might object that you can understand exactly what that line is doing,
without even working on the program. In isolation, yes, but when all the
lines in the program are like that, you soon have a mess.

Malcolm McLean wrote:
> "Ian Collins" <ian-> wrote in message [64];
>>
>>typedef struct
>>{
>> char firstname[NameLength];
>> char lastname[NameLength];
>> Currency currency;
>> float monthlySalary;
>>} Employee;
>>
>>Clear, unambiguous and not overly long.
>>
>
> But lots of snags.
>
> Firstly, it is probably better to have names stored as familair name,
> surname, other intials. I'm sure that people who work with this type of data
> have long since figured out standards. But you are not necessarily in
> control. If other parts of the application require the name as a single
> ASCIIZ string, then it is just going to be a nuisance to have endless
> conversion routines.
>
Surly one would be enough?

> Secodly, names ought to be short.
>
Name should express their intended use, if that becomes too long, the
intended use is probably wrong.

> By accesseing everythign through an
>
> Employee * employee;
>
> you have already added a layer of complexity. The rule of two states that a
> human-readable name can have two elements. So you've used your two with
>
> employee->name;
>
> employee->first_name is a parse job.
>
Is it?

> consider
>
> strcpy(new_employee->first_name,
> candiate_employees[result->success].first_name);
>
> virtually unreadable, but all we are doing is copying over a field.
>
So add a helper function.

replaceFirstName( newEmployee, candiateEmployees[result->success] );

--
Ian Collins.