1

[PATCH v2] Add command -v builtin.

Details
Message ID
<a3pzY12vCMRhjCL2DTZV42Zk8DzPU_1HPU9fsiS9arcpp1jPk2VrzaLoJGa5ZAEszSYqlRZ1o31NhJpCXuyRNNa4Cd-VfnUHhAfgzXflqzs=@protonmail.com>
DKIM signature
pass
Download raw message
Patch: +135 -0
---
 builtin/builtin.c |   1 +
 builtin/command.c | 112 ++++++++++++++++++++++++++++++++++++++++++++++
 include/builtin.h |   1 +
 meson.build       |   1 +
 test/command.sh   |  20 +++++++++
 5 files changed, 135 insertions(+)
 create mode 100644 builtin/command.c
 create mode 100644 test/command.sh

diff --git a/builtin/builtin.c b/builtin/builtin.c
index 0f0c563..20ed598 100644
--- a/builtin/builtin.c
@@ -18,6 +18,7 @@ static const struct builtin builtins[] = {
 	{ ":", builtin_colon, true },
 	{ "alias", builtin_alias, false },
 	{ "cd", builtin_cd, false },
+	{ "command", builtin_command, false },
 	{ "eval", builtin_eval, true },
 	{ "exit", builtin_exit, true },
 	{ "export", builtin_export, true },
diff --git a/builtin/command.c b/builtin/command.c
new file mode 100644
index 0000000..1884bc6
--- /dev/null
@@ -0,0 +1,112 @@
+#define _POSIX_C_SOURCE 200809L
+#include "parser.h"
+#include "mrsh/getopt.h"
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/stat.h>
+#include "mrsh/builtin.h"
+
+static const char command_usage[] = "usage: command [-v|-V|-p] command_name [argument...]\n";
+
+static int command_v(struct mrsh_state *state, const char *command_name) {
+    const char *look_alias = NULL;
+    const char *look_fn = NULL;
+    const char *path = NULL;
+    struct stat check_file;
+    size_t len_command_name = strlen(command_name);
+
+    look_alias = mrsh_hashtable_get(&state->aliases,command_name);
+    if (look_alias != NULL) {
+        printf("%s\n",look_alias);
+        return 0;
+    }
+
+    look_fn = mrsh_hashtable_get(&state->functions,command_name);
+    if (look_fn != NULL) {
+        printf("%s\n",command_name);
+        return 0;
+    }
+
+    if (mrsh_has_builtin(command_name)) {
+        printf("%s\n",command_name);
+        return 0;
+    }
+
+    for (size_t i = 0; i < keywords_len; ++i) {
+        if (strncmp(command_name, keywords[i], len_command_name) == 0) {
+            printf("%s\n",command_name);
+            return 0;
+        }
+    }
+
+    const char *default_path = "/bin:/usr/bin";  
+    char *used_path = NULL;
+
+    path = mrsh_env_get(state, "PATH", NULL);
+    if (path == NULL) {
+        fprintf(stderr, "command: $PATH doesn't exists, "
+                "searching in /bin:/usr/bin\n");
+        used_path = strdup(default_path);
+    }
+    else {
+        used_path = strdup(path);
+    }
+
+    char *full_path = NULL;
+    char *p = strtok(used_path,":");
+    while (p != NULL) {
+        full_path = malloc(strlen(p) + 1 + len_command_name + 1);
+
+        if (full_path == NULL) {
+            fprintf(stderr,"PANIC out of memory\n");
+            return 1;
+        }
+
+        strncpy(full_path,p,strlen(p));
+        full_path[strlen(p)] = '\0';
+        strncat(full_path,"/",1);
+        strncat(full_path,command_name,len_command_name);
+        
+        if (lstat(full_path,&check_file) == 0) {
+            if (check_file.st_mode & (S_IXUSR|S_IXGRP|S_IXOTH)) {
+                printf("%s\n",full_path);
+                free(full_path), full_path = NULL;
+                free(used_path), used_path = NULL;
+                return 0;
+            }
+        }
+
+        free(full_path), full_path = NULL;
+        free(used_path), used_path = NULL;
+        p = strtok(NULL,":");
+    }
+
+    return 1;
+}
+
+int builtin_command(struct mrsh_state *state, int argc, char *argv[]) {
+    mrsh_optind = 0;
+    int opt;
+    
+    while ((opt = mrsh_getopt(argc, argv, ":vVp")) != -1) {
+        switch (opt) {
+            case 'v':
+                if (argc != 3)
+                    return 1;
+                return command_v(state,argv[mrsh_optind]);
+            case 'V':
+            case 'p': 
+                fprintf(stderr, "command: `-V` and `-p` and no arg not "
+                        "yet implemented\n");
+                return 1;
+            default:
+                fprintf(stderr, "command: unknown option -- %c\n", mrsh_optopt);
+                fprintf(stderr, command_usage);
+                return 1;
+        }
+    }
+
+    return 0;
+}
+
diff --git a/include/builtin.h b/include/builtin.h
index 8a848c8..491a739 100644
--- a/include/builtin.h
+++ b/include/builtin.h
@@ -10,6 +10,7 @@ void print_escaped(const char *value);
 
 int builtin_alias(struct mrsh_state *state, int argc, char *argv[]);
 int builtin_cd(struct mrsh_state *state, int argc, char *argv[]);
+int builtin_command(struct mrsh_state *state, int argc, char *argv[]);
 int builtin_colon(struct mrsh_state *state, int argc, char *argv[]);
 int builtin_dot(struct mrsh_state *state, int argc, char *argv[]);
 int builtin_eval(struct mrsh_state *state, int argc, char *argv[]);
diff --git a/meson.build b/meson.build
index 41494c4..84931fb 100644
--- a/meson.build
+++ b/meson.build
@@ -75,6 +75,7 @@ lib_mrsh = library(
 		'builtin/alias.c',
 		'builtin/builtin.c',
 		'builtin/cd.c',
+                'builtin/command.c',
 		'builtin/colon.c',
 		'builtin/dot.c',
 		'builtin/eval.c',
diff --git a/test/command.sh b/test/command.sh
new file mode 100644
index 0000000..bb31cfa
--- /dev/null
+++ b/test/command.sh
@@ -0,0 +1,20 @@
+#!/bin/sh -e
+
+lla () {
+    ls -la
+}
+
+alias ll="ls -l"
+
+command -v if
+echo "exists if $?"
+command -v cd
+echo "exists cd $?"
+command -v ls
+echo "exists ls $?"
+command -v ll
+echo "exists ll $?"
+command -v lla
+echo "exists lla $?"
+command -v idontexists
+echo "exists idontexists $?"
-- 
2.20.1
Details
Message ID
<xIRRn5FzFGW_rSamsN6CrlW-NJj79b43upEO_VCcGhtHeS0711aAu0E1ChIk7VHQspWBM5KsNPlWKa9hQBtLLgWa1G1034fux1vhn3rqlSo=@emersion.fr>
In-Reply-To
<a3pzY12vCMRhjCL2DTZV42Zk8DzPU_1HPU9fsiS9arcpp1jPk2VrzaLoJGa5ZAEszSYqlRZ1o31NhJpCXuyRNNa4Cd-VfnUHhAfgzXflqzs=@protonmail.com> (view parent)
DKIM signature
pass
Download raw message
Hi,

Thanks for your patch! Here are some comments.

We use tabs for indentation. I see your whole patch uses spaces. Can you convert
spaces to tabs?

On Sunday, April 14, 2019 1:09 AM, dragnel <dragnel@protonmail.com> wrote:
> ---
>  builtin/builtin.c |   1 +
>  builtin/command.c | 112 ++++++++++++++++++++++++++++++++++++++++++++++
>  include/builtin.h |   1 +
>  meson.build       |   1 +
>  test/command.sh   |  20 +++++++++
>  5 files changed, 135 insertions(+)
>  create mode 100644 builtin/command.c
>  create mode 100644 test/command.sh
>
> diff --git a/builtin/builtin.c b/builtin/builtin.c
> index 0f0c563..20ed598 100644
> --- a/builtin/builtin.c
> +++ b/builtin/builtin.c
> @@ -18,6 +18,7 @@ static const struct builtin builtins[] = {
>  	{ ":", builtin_colon, true },
>  	{ "alias", builtin_alias, false },
>  	{ "cd", builtin_cd, false },
> +	{ "command", builtin_command, false },
>  	{ "eval", builtin_eval, true },
>  	{ "exit", builtin_exit, true },
>  	{ "export", builtin_export, true },
> diff --git a/builtin/command.c b/builtin/command.c
> new file mode 100644
> index 0000000..1884bc6
> --- /dev/null
> +++ b/builtin/command.c
> @@ -0,0 +1,112 @@
> +#define _POSIX_C_SOURCE 200809L
> +#include "parser.h"
> +#include "mrsh/getopt.h"
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <sys/stat.h>
> +#include "mrsh/builtin.h"
> +
> +static const char command_usage[] = "usage: command [-v|-V|-p] command_name [argument...]\n";

This string is a little bit long. It's possible to split it like so:

    static const char command_usage[] = "usage: command etc etc etc "
    	"etc etc etc\n";

> +static int command_v(struct mrsh_state *state, const char *command_name) {
> +    const char *look_alias = NULL;

Please declare variables when they are used for the first time (e.g. for
look_alias, it would be when mrsh_hashtable_get is called).

> +    const char *look_fn = NULL;
> +    const char *path = NULL;
> +    struct stat check_file;
> +    size_t len_command_name = strlen(command_name);
> +
> +    look_alias = mrsh_hashtable_get(&state->aliases,command_name);
> +    if (look_alias != NULL) {
> +        printf("%s\n",look_alias);
> +        return 0;
> +    }
> +
> +    look_fn = mrsh_hashtable_get(&state->functions,command_name);
> +    if (look_fn != NULL) {
> +        printf("%s\n",command_name);
> +        return 0;
> +    }
> +
> +    if (mrsh_has_builtin(command_name)) {
> +        printf("%s\n",command_name);
> +        return 0;
> +    }
> +
> +    for (size_t i = 0; i < keywords_len; ++i) {
> +        if (strncmp(command_name, keywords[i], len_command_name) == 0) {

I'd rather use strcmp here.

> +            printf("%s\n",command_name);
> +            return 0;
> +        }
> +    }
> +
> +    const char *default_path = "/bin:/usr/bin";
> +    char *used_path = NULL;
> +
> +    path = mrsh_env_get(state, "PATH", NULL);
> +    if (path == NULL) {
> +        fprintf(stderr, "command: $PATH doesn't exists, "
> +                "searching in /bin:/usr/bin\n");
> +        used_path = strdup(default_path);
> +    }
> +    else {
> +        used_path = strdup(path);
> +    }
> +
> +    char *full_path = NULL;
> +    char *p = strtok(used_path,":");
> +    while (p != NULL) {
> +        full_path = malloc(strlen(p) + 1 + len_command_name + 1);
> +
> +        if (full_path == NULL) {
> +            fprintf(stderr,"PANIC out of memory\n");
> +            return 1;
> +        }
> +
> +        strncpy(full_path,p,strlen(p));
> +        full_path[strlen(p)] = '\0';
> +        strncat(full_path,"/",1);
> +        strncat(full_path,command_name,len_command_name);
> +
> +        if (lstat(full_path,&check_file) == 0) {
> +            if (check_file.st_mode & (S_IXUSR|S_IXGRP|S_IXOTH)) {
> +                printf("%s\n",full_path);
> +                free(full_path), full_path = NULL;
> +                free(used_path), used_path = NULL;
> +                return 0;
> +            }
> +        }
> +
> +        free(full_path), full_path = NULL;
> +        free(used_path), used_path = NULL;
> +        p = strtok(NULL,":");
> +    }

Hmm. I think we already have code for this, see expand_path in shell/path.c.

> +    return 1;
> +}
> +
> +int builtin_command(struct mrsh_state *state, int argc, char *argv[]) {
> +    mrsh_optind = 0;
> +    int opt;
> +
> +    while ((opt = mrsh_getopt(argc, argv, ":vVp")) != -1) {
> +        switch (opt) {
> +            case 'v':

Style: `case` should have the same indentation level as `switch`.

> +                if (argc != 3)
> +                    return 1;
> +                return command_v(state,argv[mrsh_optind]);
> +            case 'V':
> +            case 'p':
> +                fprintf(stderr, "command: `-V` and `-p` and no arg not "
> +                        "yet implemented\n");
> +                return 1;
> +            default:
> +                fprintf(stderr, "command: unknown option -- %c\n", mrsh_optopt);
> +                fprintf(stderr, command_usage);
> +                return 1;
> +        }
> +    }
> +
> +    return 0;
> +}
> +
> diff --git a/include/builtin.h b/include/builtin.h
> index 8a848c8..491a739 100644
> --- a/include/builtin.h
> +++ b/include/builtin.h
> @@ -10,6 +10,7 @@ void print_escaped(const char *value);
>
>  int builtin_alias(struct mrsh_state *state, int argc, char *argv[]);
>  int builtin_cd(struct mrsh_state *state, int argc, char *argv[]);
> +int builtin_command(struct mrsh_state *state, int argc, char *argv[]);
>  int builtin_colon(struct mrsh_state *state, int argc, char *argv[]);
>  int builtin_dot(struct mrsh_state *state, int argc, char *argv[]);
>  int builtin_eval(struct mrsh_state *state, int argc, char *argv[]);
> diff --git a/meson.build b/meson.build
> index 41494c4..84931fb 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -75,6 +75,7 @@ lib_mrsh = library(
>  		'builtin/alias.c',
>  		'builtin/builtin.c',
>  		'builtin/cd.c',
> +                'builtin/command.c',
>  		'builtin/colon.c',
>  		'builtin/dot.c',
>  		'builtin/eval.c',
> diff --git a/test/command.sh b/test/command.sh
> new file mode 100644
> index 0000000..bb31cfa
> --- /dev/null
> +++ b/test/command.sh
> @@ -0,0 +1,20 @@
> +#!/bin/sh -e
> +
> +lla () {
> +    ls -la
> +}
> +
> +alias ll="ls -l"
> +
> +command -v if
> +echo "exists if $?"
> +command -v cd
> +echo "exists cd $?"
> +command -v ls
> +echo "exists ls $?"
> +command -v ll
> +echo "exists ll $?"
> +command -v lla
> +echo "exists lla $?"
> +command -v idontexists
> +echo "exists idontexists $?"

Adding a test is a good idea :)

Can you add it to the test/meson.build file, so that it is run with
`ninja test`?