Re: [hackers] [PATCH][sbase] Add patch(1)
On Sun, Sep 24, 2017 at 8:57 PM, Mattias Andrée wrote: > On Sun, 24 Sep 2017 19:24:10 +0200 > Silvan Jegen wrote: > >> Heyho >> >> On Sun, Sep 24, 2017 at 06:28:57PM +0200, Mattias Andrée wrote: >> > On Sun, 24 Sep 2017 14:08:41 +0200 >> > Silvan Jegen wrote: >> > >> > > > + >> > > > + if (!new->len) >> > > > + for (i = 0; i < old->len; i++) >> > > > + if (old->lines[i].data[-2] != '-') >> > > >> > > I think according to the standard, refering to data[-2] invokes undefined >> > > behaviour, since at this time, data points to the beginning of the array. >> > >> > I'm not sure whether it is defined or undefined; I would think that it >> > defined, but that adding integers larger than INTPTR_MAX is undefined. >> > I will change to `*(data - 2)` as this is clearly defined. >> >> I was referring to >> https://stackoverflow.com/questions/3473675/are-negative-array-indexes-allowed-in-c/3473686#3473686 >> . `*(data -2) is equivalent to 'data[-2]' but since 'data' doesn't point >> to the second element of the array, I don't think this is valid. > > Hi! > > I think there has been some misunderstanding here, > and that we are in agreement that `a[-b]` in it self > is not invalid, but that question is whether the > deferenced address is valid. I understand why this > looks incorrect, `old->lines->data[0]` does not > actually point to the first character on a line > in a line but rather to the first character in the > line that is part of the content of the file that > hunk patches. For example if the patchfile contains > the line "- abc", `old->lines->data[0]` is `a`, not > `-`, because "- " part of the annotations in the > hunk. Ah, I missed that. In that case this negative index is ok. > This should probably be clarified, but you can see > that this happening just above this code. Pointing this out in a comment seems like a good idea to me. > I will look that your other comments later. Sure! Cheers, Silvan
Re: [hackers] [PATCH][sbase] Add patch(1)
On Sun, 24 Sep 2017 19:24:10 +0200 Silvan Jegen wrote: > Heyho > > On Sun, Sep 24, 2017 at 06:28:57PM +0200, Mattias Andrée wrote: > > On Sun, 24 Sep 2017 14:08:41 +0200 > > Silvan Jegen wrote: > > > > > > + > > > > + if (!new->len) > > > > + for (i = 0; i < old->len; i++) > > > > + if (old->lines[i].data[-2] != '-') > > > > > > I think according to the standard, refering to data[-2] invokes undefined > > > behaviour, since at this time, data points to the beginning of the array. > > > > > > > I'm not sure whether it is defined or undefined; I would think that it > > defined, but that adding integers larger than INTPTR_MAX is undefined. > > I will change to `*(data - 2)` as this is clearly defined. > > I was referring to > https://stackoverflow.com/questions/3473675/are-negative-array-indexes-allowed-in-c/3473686#3473686 > . `*(data -2) is equivalent to 'data[-2]' but since 'data' doesn't point > to the second element of the array, I don't think this is valid. Hi! I think there has been some misunderstanding here, and that we are in agreement that `a[-b]` in it self is not invalid, but that question is whether the deferenced address is valid. I understand why this looks incorrect, `old->lines->data[0]` does not actually point to the first character on a line in a line but rather to the first character in the line that is part of the content of the file that hunk patches. For example if the patchfile contains the line "- abc", `old->lines->data[0]` is `a`, not `-`, because "- " part of the annotations in the hunk. This should probably be clarified, but you can see that this happening just above this code. I will look that your other comments later. pgpWxkIdgIKQz.pgp Description: OpenPGP digital signature
Re: [hackers] [PATCH][sbase] Add patch(1)
Heyho
On Sun, Sep 24, 2017 at 06:28:57PM +0200, Mattias Andrée wrote:
> On Sun, 24 Sep 2017 14:08:41 +0200
> Silvan Jegen wrote:
>
> > Heyho Mattias!
> >
> > I had a look at the patch. It's a lot of code (still only about 1/3 of
> > GNU's patch size though) and it was rather hard for me to follow so more
> > review should be done. Find my comments below.
> >
> > On Sun, Sep 03, 2017 at 07:13:20PM +0200, Mattias Andrée wrote:
> > > +static void
> > > +save_file_cpp(FILE *f, struct file_data *file)
> > > +{
> > > + size_t i, j, n;
> > > + char annot = ' ';
> > > +
> > > + for (i = 0; i <= file->n; i++) {
> > > + if ((n = file->d[i].nold)) {
> >
> > In other places you iterate with "i < file->n" (see save_file below for
> > example) so I think this may be an off-by-one error.
>
> There is an if-statement, that breaks the loop if `i == file->`,
> after this clause, so this should be correct. I'll add blank lines
I saw the break statement but the 'n = file->d[i].nold' will be executed before
reaching that break statement resulting in a segfault when i == file->n.
> around that if-statement to make it clearer.
>
> >
> >
> > > + fprintf(f, "%s\n", annot == '+' ? "#else" : ifndef);
> > > + for (j = 0; j < n; j++) {
> > > + fwriteline(f, file->d[i].old[j]);
> > > + if (missinglf(file->d[i].old[j]))
> > > + fprintf(f, "\n");
> > > + }
> > > + annot = '-';
> > > + }
> > > + if (i == file->n)
> > > + break;
> > > + if (annot == '-')
> > > + fprintf(f, "%s\n", file->d[i].new ? "#else" : "#endif");
> > > + else if (annot == ' ' && file->d[i].new)
> > > + fprintf(f, "%s\n", ifdef);
> > > + else if (annot == '+' && !file->d[i].new)
> > > + fprintf(f, "#endif\n");
> > > + fwriteline(f, file->d[i].line);
> > > + if ((i + 1 < file->n || file->d[i].new) &&
> > > missinglf(file->d[i].line))
> > > + fprintf(f, "\n");
> > > + annot = file->d[i].new ? '+' : ' ';
> > > + }
> > > + if (annot != ' ')
> > > + fprintf(f, "#endif\n");
> > > +}
> > > +
> > > +static void
> > > +parse_hunk_copied(struct hunk *hunk, struct parsed_hunk *parsed)
> > > +{
> > > + struct hunk_content *old = &parsed->old, *new = &parsed->new;
> > > + size_t i = 0, a, b;
> > > + char *p;
> > > +
> > > + free(hunk->head->data);
> > > +
> > > + old->lines = enmalloc(FAILURE, hunk->nlines * sizeof(*old->lines));
> > > + new->lines = enmalloc(FAILURE, hunk->nlines * sizeof(*new->lines));
> > > + parsed->annot = enmalloc(FAILURE, hunk->nlines + 1);
> > > +
> > > + p = hunk->lines[i++].data + 4;
> > > + old->start = strtoul(p, &p, 10);
> > > + old->len = 0;
> > > +
> > > + for (; hunk->lines[i].data[1] == ' '; i++)
> > > + subline(old->lines + old->len++, hunk->lines + i, 2);
> > > +
> > > + p = hunk->lines[i++].data + 4;
> > > + new->start = strtoul(p, &p, 10);
> > > + new->len = 0;
> > > +
> > > + if (old->len) {
> > > + for (; i < hunk->nlines; i++)
> > > + subline(new->lines + new->len++, hunk->lines + i, 2);
> > > + } else {
> > > + for (; i < hunk->nlines; i++) {
> > > + subline(new->lines + new->len++, hunk->lines + i, 2);
> > > + if (hunk->lines[i].data[0] != '+')
> > > + subline(old->lines + old->len++, hunk->lines +
> > > i, 2);
> > > + }
> > > + }
> >
> > I think this if-else block can be rewritten like this.
> >
> > for (; i < hunk->nlines; i++) {
> > subline(new->lines + new->len++, hunk->lines + i, 2);
> > if (old->len == 0 && hunk->lines[i].data[0] != '+')
> > subline(old->lines + old->len++, hunk->lines + i, 2);
> > }
>
> I will use `!old->len` instead of `old->len == 0`.
>
> >
> >
> > > +
> > > + if (!new->len)
> > > + for (i = 0; i < old->len; i++)
> > > + if (old->lines[i].data[-2] != '-')
> >
> > I think according to the standard, refering to data[-2] invokes undefined
> > behaviour, since at this time, data points to the beginning of the array.
>
> I'm not sure whether it is defined or undefined; I would think that it
> defined, but that adding integers larger than INTPTR_MAX is undefined.
> I will change to `*(data - 2)` as this is clearly defined.
I was referring to
https://stackoverflow.com/questions/3473675/are-negative-array-indexes-allowed-in-c/3473686#3473686
. `*(data -2) is equivalent to 'data[-2]' but since 'data' doesn't point
to the second element of the array, I don't think this is valid.
> >
> >
> > > + new->lines[new->len++] = old->lines[i];
> > > +
> > > +#define OLDLINE a < old->len && old->lines[a].data[-2]
> > > +#define NEWLINE b < new->len && new->lines[b].data[-2]
> > > +
> > > + for (i = a
Re: [hackers] [PATCH][sbase] Add patch(1)
On Sun, 24 Sep 2017 14:08:41 +0200
Silvan Jegen wrote:
> Heyho Mattias!
>
> I had a look at the patch. It's a lot of code (still only about 1/3 of
> GNU's patch size though) and it was rather hard for me to follow so more
> review should be done. Find my comments below.
>
> On Sun, Sep 03, 2017 at 07:13:20PM +0200, Mattias Andrée wrote:
> > +static void
> > +save_file_cpp(FILE *f, struct file_data *file)
> > +{
> > + size_t i, j, n;
> > + char annot = ' ';
> > +
> > + for (i = 0; i <= file->n; i++) {
> > + if ((n = file->d[i].nold)) {
>
> In other places you iterate with "i < file->n" (see save_file below for
> example) so I think this may be an off-by-one error.
There is an if-statement, that breaks the loop if `i == file->`,
after this clause, so this should be correct. I'll add blank lines
around that if-statement to make it clearer.
>
>
> > + fprintf(f, "%s\n", annot == '+' ? "#else" : ifndef);
> > + for (j = 0; j < n; j++) {
> > + fwriteline(f, file->d[i].old[j]);
> > + if (missinglf(file->d[i].old[j]))
> > + fprintf(f, "\n");
> > + }
> > + annot = '-';
> > + }
> > + if (i == file->n)
> > + break;
> > + if (annot == '-')
> > + fprintf(f, "%s\n", file->d[i].new ? "#else" : "#endif");
> > + else if (annot == ' ' && file->d[i].new)
> > + fprintf(f, "%s\n", ifdef);
> > + else if (annot == '+' && !file->d[i].new)
> > + fprintf(f, "#endif\n");
> > + fwriteline(f, file->d[i].line);
> > + if ((i + 1 < file->n || file->d[i].new) &&
> > missinglf(file->d[i].line))
> > + fprintf(f, "\n");
> > + annot = file->d[i].new ? '+' : ' ';
> > + }
> > + if (annot != ' ')
> > + fprintf(f, "#endif\n");
> > +}
> > +
> > +static void
> > +parse_hunk_copied(struct hunk *hunk, struct parsed_hunk *parsed)
> > +{
> > + struct hunk_content *old = &parsed->old, *new = &parsed->new;
> > + size_t i = 0, a, b;
> > + char *p;
> > +
> > + free(hunk->head->data);
> > +
> > + old->lines = enmalloc(FAILURE, hunk->nlines * sizeof(*old->lines));
> > + new->lines = enmalloc(FAILURE, hunk->nlines * sizeof(*new->lines));
> > + parsed->annot = enmalloc(FAILURE, hunk->nlines + 1);
> > +
> > + p = hunk->lines[i++].data + 4;
> > + old->start = strtoul(p, &p, 10);
> > + old->len = 0;
> > +
> > + for (; hunk->lines[i].data[1] == ' '; i++)
> > + subline(old->lines + old->len++, hunk->lines + i, 2);
> > +
> > + p = hunk->lines[i++].data + 4;
> > + new->start = strtoul(p, &p, 10);
> > + new->len = 0;
> > +
> > + if (old->len) {
> > + for (; i < hunk->nlines; i++)
> > + subline(new->lines + new->len++, hunk->lines + i, 2);
> > + } else {
> > + for (; i < hunk->nlines; i++) {
> > + subline(new->lines + new->len++, hunk->lines + i, 2);
> > + if (hunk->lines[i].data[0] != '+')
> > + subline(old->lines + old->len++, hunk->lines +
> > i, 2);
> > + }
> > + }
>
> I think this if-else block can be rewritten like this.
>
> for (; i < hunk->nlines; i++) {
> subline(new->lines + new->len++, hunk->lines + i, 2);
> if (old->len == 0 && hunk->lines[i].data[0] != '+')
> subline(old->lines + old->len++, hunk->lines + i, 2);
> }
I will use `!old->len` instead of `old->len == 0`.
>
>
> > +
> > + if (!new->len)
> > + for (i = 0; i < old->len; i++)
> > + if (old->lines[i].data[-2] != '-')
>
> I think according to the standard, refering to data[-2] invokes undefined
> behaviour, since at this time, data points to the beginning of the array.
I'm not sure whether it is defined or undefined; I would think that it
defined, but that adding integers larger than INTPTR_MAX is undefined.
I will change to `*(data - 2)` as this is clearly defined.
>
>
> > + new->lines[new->len++] = old->lines[i];
> > +
> > +#define OLDLINE a < old->len && old->lines[a].data[-2]
> > +#define NEWLINE b < new->len && new->lines[b].data[-2]
> > +
> > + for (i = a = b = 0; a < old->len || b < new->len;) {
> > + if (OLDLINE == '-') parsed->annot[i++] = '-', a++;
> > + if (NEWLINE == '+') parsed->annot[i++] = '+', b++;
> > + while (OLDLINE == ' ' && NEWLINE == ' ')
> > + parsed->annot[i++] = ' ', a++, b++;
> > + while (OLDLINE == '!') parsed->annot[i++] = '<', a++;
> > + while (NEWLINE == '!') parsed->annot[i++] = '>', b++;
> > + }
> > + parsed->annot[i] = 0;
> > +}
> > +
> > +static void
> > +apply_contiguous_edit(struct file_data *f, size_t ln, size_t rm, size_t
> > ad, struct li
Re: [hackers] [PATCH][sbase] Add patch(1)
Heyho Mattias!
I had a look at the patch. It's a lot of code (still only about 1/3 of
GNU's patch size though) and it was rather hard for me to follow so more
review should be done. Find my comments below.
On Sun, Sep 03, 2017 at 07:13:20PM +0200, Mattias Andrée wrote:
> Signed-off-by: Mattias Andrée
> ---
> Makefile |2 +
> README |1 +
> TODO |1 -
> libutil/asprintf.c | 74 +++
> libutil/getlines.c | 17 +-
> patch.1| 250 +++
> patch.c| 1835
>
> text.h |4 +-
> util.h |5 +
> 9 files changed, 2182 insertions(+), 7 deletions(-)
> create mode 100644 libutil/asprintf.c
> create mode 100644 patch.1
> create mode 100644 patch.c
>
> diff --git a/Makefile b/Makefile
> index 1c39fef..014db74 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -45,6 +45,7 @@ LIBUTFSRC =\
>
> LIBUTIL = libutil.a
> LIBUTILSRC =\
> + libutil/asprintf.c\
> libutil/concat.c\
> libutil/cp.c\
> libutil/crypt.c\
> @@ -132,6 +133,7 @@ BIN =\
> nohup\
> od\
> paste\
> + patch\
> pathchk\
> printenv\
> printf\
> diff --git a/README b/README
> index da2e500..6c94f2f 100644
> --- a/README
> +++ b/README
> @@ -59,6 +59,7 @@ The following tools are implemented:
> 0#*|o nl .
> 0=*|o nohup .
> 0=*|o od .
> +0=patch .
> 0#* o pathchk .
> #*|o paste .
> 0=*|x printenv.
> diff --git a/TODO b/TODO
> index 5edb8a3..fe2344e 100644
> --- a/TODO
> +++ b/TODO
> @@ -8,7 +8,6 @@ awk
> bc
> diff
> ed manpage
> -patch
> stty
>
> If you are looking for some work to do on sbase, another option is to
> diff --git a/libutil/asprintf.c b/libutil/asprintf.c
> new file mode 100644
> index 000..929ed09
> --- /dev/null
> +++ b/libutil/asprintf.c
> @@ -0,0 +1,74 @@
> +/* See LICENSE file for copyright and license details. */
> +#include
> +#include
> +#include
> +
> +#include "../util.h"
> +
> +static int xenvasprintf(int, char **, const char *, va_list);
> +
> +int
> +asprintf(char **strp, const char *fmt, ...)
> +{
> + va_list ap;
> + int ret;
> +
> + va_start(ap, fmt);
> + ret = xenvasprintf(-1, strp, fmt, ap);
> + va_end(ap);
> +
> + return ret;
> +}
> +
> +int
> +easprintf(char **strp, const char *fmt, ...)
> +{
> + va_list ap;
> + int ret;
> +
> + va_start(ap, fmt);
> + ret = xenvasprintf(1, strp, fmt, ap);
> + va_end(ap);
> +
> + return ret;
> +}
> +
> +int
> +enasprintf(int status, char **strp, const char *fmt, ...)
> +{
> + va_list ap;
> + int ret;
> +
> + va_start(ap, fmt);
> + ret = xenvasprintf(status, strp, fmt, ap);
> + va_end(ap);
> +
> + return ret;
> +}
> +
> +int
> +xenvasprintf(int status, char **strp, const char *fmt, va_list ap)
> +{
> + int ret;
> + va_list ap2;
> +
> + va_copy(ap2, ap);
> + ret = vsnprintf(0, 0, fmt, ap2);
> + va_end(ap2);
> + if (ret < 0) {
> + if (status >= 0)
> + enprintf(status, "vsnprintf:");
> + *strp = 0;
> + return -1;
> + }
> +
> + *strp = malloc(ret + 1);
> + if (!*strp) {
> + if (status >= 0)
> + enprintf(status, "malloc:");
> + return -1;
> + }
> +
> + vsprintf(*strp, fmt, ap);
> + return ret;
> +}
> diff --git a/libutil/getlines.c b/libutil/getlines.c
> index b912769..9af7684 100644
> --- a/libutil/getlines.c
> +++ b/libutil/getlines.c
> @@ -7,7 +7,7 @@
> #include "../util.h"
>
> void
> -getlines(FILE *fp, struct linebuf *b)
> +ngetlines(int status, FILE *fp, struct linebuf *b)
> {
> char *line = NULL;
> size_t size = 0, linelen = 0;
> @@ -16,17 +16,24 @@ getlines(FILE *fp, struct linebuf *b)
> while ((len = getline(&line, &size, fp)) > 0) {
> if (++b->nlines > b->capacity) {
> b->capacity += 512;
> - b->lines = erealloc(b->lines, b->capacity *
> sizeof(*b->lines));
> + b->lines = enrealloc(status, b->lines, b->capacity *
> sizeof(*b->lines));
> }
> linelen = len;
> - b->lines[b->nlines - 1].data = memcpy(emalloc(linelen + 1),
> line, linelen + 1);
> + b->lines[b->nlines - 1].data = memcpy(enmalloc(status, linelen
> + 1), line, linelen + 1);
> b->lines[b->nlines - 1].len = linelen;
> }
> free(line);
> - if (b->lines && b->nlines && linelen && b->lines[b->nlines -
> 1].data[linelen - 1] != '\n') {
> - b->lines[b->nlines - 1].data = erealloc(b->lines[b->nlines -
> 1].data, linelen + 2);
> + b->nolf = b->lines && b->nlines && linelen && b->lines[b->nlines -
> 1].data[linelen - 1] != '\n';
> + if (b->nolf) {
> + b->lines[b->nlines - 1].data = enrealloc(status,
Re: [hackers] [PATCH][sbase] Add patch(1)
On Mon, Sep 11, 2017 at 08:57:02PM +0200, Mattias Andrée wrote: > On Mon, 11 Sep 2017 20:09:33 +0200 > Silvan Jegen wrote: > > >> +when comparing directories. If however, the > > > > There should probably be an additional comma like this: > > > > "If, however, the file..." > > I think “However, if the file...” is better. Sounds good to me! > > > +portion of a patch. A patch is a signal > > > +file-comparison output from > > > > Not sure what a "signal file-comparison output" is... is this official > > POSIX/patch terminology? > > s/signal/singel/, so a patch file includes > a patchset with is a number of patches, one > per file in the patch file. Haha, I totally didn't think of that... > >> +Symbolic links are treated as regular files, > >> +provided that they lead to regular files. > > > > maybe s/lead/link/ ? > > Not sure, perhaps “link” sounds more natural, > but for me, ”link” means we are talking about > the step and not the final step when following > the link. However, I will add this change. I think both are understandable but the term "link" for symbolic links is definitely the more common one, I would say. signature.asc Description: PGP signature
Re: [hackers] [PATCH][sbase] Add patch(1)
On Mon, 11 Sep 2017 20:09:33 +0200 Silvan Jegen wrote: >> +when comparing directories. If however, the > > There should probably be an additional comma like this: > > "If, however, the file..." I think “However, if the file...” is better. > > +portion of a patch. A patch is a signal > > +file-comparison output from > > Not sure what a "signal file-comparison output" is... is this official > POSIX/patch terminology? s/signal/singel/, so a patch file includes a patchset with is a number of patches, one per file in the patch file. >> +Symbolic links are treated as regular files, >> +provided that they lead to regular files. > > maybe s/lead/link/ ? Not sure, perhaps “link” sounds more natural, but for me, ”link” means we are talking about the step and not the final step when following the link. However, I will add this change. pgpFOH92wIueQ.pgp Description: OpenPGP digital signature
Re: [hackers] [PATCH][sbase] Add patch(1)
Hi Mattias!
Thanks for (the) patch! :P
Some comments below. For now I only managed to look at whitespace issues
in the patch and suggest some corrections for spelling/grammar issues
in the man page text. I hope to get around looking at the code in the
near future.
On Sun, Sep 03, 2017 at 07:13:20PM +0200, Mattias Andrée wrote:
> Signed-off-by: Mattias Andrée
> ---
> Makefile |2 +
> README |1 +
> TODO |1 -
> libutil/asprintf.c | 74 +++
> libutil/getlines.c | 17 +-
> patch.1| 250 +++
> patch.c| 1835
>
> text.h |4 +-
> util.h |5 +
> 9 files changed, 2182 insertions(+), 7 deletions(-)
> create mode 100644 libutil/asprintf.c
> create mode 100644 patch.1
> create mode 100644 patch.c
>
> diff --git a/Makefile b/Makefile
> index 1c39fef..014db74 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -45,6 +45,7 @@ LIBUTFSRC =\
>
> LIBUTIL = libutil.a
> LIBUTILSRC =\
> + libutil/asprintf.c\
> libutil/concat.c\
> libutil/cp.c\
> libutil/crypt.c\
> @@ -132,6 +133,7 @@ BIN =\
> nohup\
> od\
> paste\
> + patch\
> pathchk\
> printenv\
> printf\
> diff --git a/README b/README
> index da2e500..6c94f2f 100644
> --- a/README
> +++ b/README
> @@ -59,6 +59,7 @@ The following tools are implemented:
> 0#*|o nl .
> 0=*|o nohup .
> 0=*|o od .
> +0=patch .
> 0#* o pathchk .
> #*|o paste .
> 0=*|x printenv.
> diff --git a/TODO b/TODO
> index 5edb8a3..fe2344e 100644
> --- a/TODO
> +++ b/TODO
> @@ -8,7 +8,6 @@ awk
> bc
> diff
> ed manpage
> -patch
> stty
>
> If you are looking for some work to do on sbase, another option is to
> diff --git a/libutil/asprintf.c b/libutil/asprintf.c
> new file mode 100644
> index 000..929ed09
> --- /dev/null
> +++ b/libutil/asprintf.c
> @@ -0,0 +1,74 @@
> +/* See LICENSE file for copyright and license details. */
> +#include
> +#include
> +#include
> +
> +#include "../util.h"
> +
> +static int xenvasprintf(int, char **, const char *, va_list);
> +
> +int
> +asprintf(char **strp, const char *fmt, ...)
> +{
> + va_list ap;
> + int ret;
> +
> + va_start(ap, fmt);
> + ret = xenvasprintf(-1, strp, fmt, ap);
> + va_end(ap);
> +
> + return ret;
> +}
> +
> +int
> +easprintf(char **strp, const char *fmt, ...)
> +{
> + va_list ap;
> + int ret;
> +
> + va_start(ap, fmt);
> + ret = xenvasprintf(1, strp, fmt, ap);
> + va_end(ap);
> +
> + return ret;
> +}
> +
> +int
> +enasprintf(int status, char **strp, const char *fmt, ...)
> +{
> + va_list ap;
> + int ret;
> +
> + va_start(ap, fmt);
> + ret = xenvasprintf(status, strp, fmt, ap);
> + va_end(ap);
> +
> + return ret;
> +}
> +
> +int
> +xenvasprintf(int status, char **strp, const char *fmt, va_list ap)
> +{
> + int ret;
> + va_list ap2;
> +
> + va_copy(ap2, ap);
> + ret = vsnprintf(0, 0, fmt, ap2);
> + va_end(ap2);
> + if (ret < 0) {
> + if (status >= 0)
> + enprintf(status, "vsnprintf:");
> + *strp = 0;
> + return -1;
> + }
> +
> + *strp = malloc(ret + 1);
> + if (!*strp) {
> + if (status >= 0)
> + enprintf(status, "malloc:");
> + return -1;
> + }
> +
> + vsprintf(*strp, fmt, ap);
> + return ret;
> +}
> diff --git a/libutil/getlines.c b/libutil/getlines.c
> index b912769..9af7684 100644
> --- a/libutil/getlines.c
> +++ b/libutil/getlines.c
> @@ -7,7 +7,7 @@
> #include "../util.h"
>
> void
> -getlines(FILE *fp, struct linebuf *b)
> +ngetlines(int status, FILE *fp, struct linebuf *b)
> {
> char *line = NULL;
> size_t size = 0, linelen = 0;
> @@ -16,17 +16,24 @@ getlines(FILE *fp, struct linebuf *b)
> while ((len = getline(&line, &size, fp)) > 0) {
> if (++b->nlines > b->capacity) {
> b->capacity += 512;
> - b->lines = erealloc(b->lines, b->capacity *
> sizeof(*b->lines));
> + b->lines = enrealloc(status, b->lines, b->capacity *
> sizeof(*b->lines));
> }
> linelen = len;
> - b->lines[b->nlines - 1].data = memcpy(emalloc(linelen + 1),
> line, linelen + 1);
> + b->lines[b->nlines - 1].data = memcpy(enmalloc(status, linelen
> + 1), line, linelen + 1);
> b->lines[b->nlines - 1].len = linelen;
> }
> free(line);
> - if (b->lines && b->nlines && linelen && b->lines[b->nlines -
> 1].data[linelen - 1] != '\n') {
> - b->lines[b->nlines - 1].data = erealloc(b->lines[b->nlines -
> 1].data, linelen + 2);
> + b->nolf = b->lines && b->nlines && linelen && b->lines[b->nlines -
> 1].data[linelen - 1] != '\n';
> + if (b->nolf) {
> +
Re: [hackers] [PATCH][sbase] Add patch(1)
On Sun, 3 Sep 2017 19:13:20 +0200 Mattias Andrée wrote: Dear Mattias, > I saw that you had sent the patch(1) patch already back in March 2016. Hopefully we can review it accordingly now and Michael Forney merge it! With best regards Laslo Hunhold -- Laslo Hunhold
[hackers] [PATCH][sbase] Add patch(1)
Signed-off-by: Mattias Andrée
---
Makefile |2 +
README |1 +
TODO |1 -
libutil/asprintf.c | 74 +++
libutil/getlines.c | 17 +-
patch.1| 250 +++
patch.c| 1835
text.h |4 +-
util.h |5 +
9 files changed, 2182 insertions(+), 7 deletions(-)
create mode 100644 libutil/asprintf.c
create mode 100644 patch.1
create mode 100644 patch.c
diff --git a/Makefile b/Makefile
index 1c39fef..014db74 100644
--- a/Makefile
+++ b/Makefile
@@ -45,6 +45,7 @@ LIBUTFSRC =\
LIBUTIL = libutil.a
LIBUTILSRC =\
+ libutil/asprintf.c\
libutil/concat.c\
libutil/cp.c\
libutil/crypt.c\
@@ -132,6 +133,7 @@ BIN =\
nohup\
od\
paste\
+ patch\
pathchk\
printenv\
printf\
diff --git a/README b/README
index da2e500..6c94f2f 100644
--- a/README
+++ b/README
@@ -59,6 +59,7 @@ The following tools are implemented:
0#*|o nl .
0=*|o nohup .
0=*|o od .
+0=patch .
0#* o pathchk .
#*|o paste .
0=*|x printenv.
diff --git a/TODO b/TODO
index 5edb8a3..fe2344e 100644
--- a/TODO
+++ b/TODO
@@ -8,7 +8,6 @@ awk
bc
diff
ed manpage
-patch
stty
If you are looking for some work to do on sbase, another option is to
diff --git a/libutil/asprintf.c b/libutil/asprintf.c
new file mode 100644
index 000..929ed09
--- /dev/null
+++ b/libutil/asprintf.c
@@ -0,0 +1,74 @@
+/* See LICENSE file for copyright and license details. */
+#include
+#include
+#include
+
+#include "../util.h"
+
+static int xenvasprintf(int, char **, const char *, va_list);
+
+int
+asprintf(char **strp, const char *fmt, ...)
+{
+ va_list ap;
+ int ret;
+
+ va_start(ap, fmt);
+ ret = xenvasprintf(-1, strp, fmt, ap);
+ va_end(ap);
+
+ return ret;
+}
+
+int
+easprintf(char **strp, const char *fmt, ...)
+{
+ va_list ap;
+ int ret;
+
+ va_start(ap, fmt);
+ ret = xenvasprintf(1, strp, fmt, ap);
+ va_end(ap);
+
+ return ret;
+}
+
+int
+enasprintf(int status, char **strp, const char *fmt, ...)
+{
+ va_list ap;
+ int ret;
+
+ va_start(ap, fmt);
+ ret = xenvasprintf(status, strp, fmt, ap);
+ va_end(ap);
+
+ return ret;
+}
+
+int
+xenvasprintf(int status, char **strp, const char *fmt, va_list ap)
+{
+ int ret;
+ va_list ap2;
+
+ va_copy(ap2, ap);
+ ret = vsnprintf(0, 0, fmt, ap2);
+ va_end(ap2);
+ if (ret < 0) {
+ if (status >= 0)
+ enprintf(status, "vsnprintf:");
+ *strp = 0;
+ return -1;
+ }
+
+ *strp = malloc(ret + 1);
+ if (!*strp) {
+ if (status >= 0)
+ enprintf(status, "malloc:");
+ return -1;
+ }
+
+ vsprintf(*strp, fmt, ap);
+ return ret;
+}
diff --git a/libutil/getlines.c b/libutil/getlines.c
index b912769..9af7684 100644
--- a/libutil/getlines.c
+++ b/libutil/getlines.c
@@ -7,7 +7,7 @@
#include "../util.h"
void
-getlines(FILE *fp, struct linebuf *b)
+ngetlines(int status, FILE *fp, struct linebuf *b)
{
char *line = NULL;
size_t size = 0, linelen = 0;
@@ -16,17 +16,24 @@ getlines(FILE *fp, struct linebuf *b)
while ((len = getline(&line, &size, fp)) > 0) {
if (++b->nlines > b->capacity) {
b->capacity += 512;
- b->lines = erealloc(b->lines, b->capacity *
sizeof(*b->lines));
+ b->lines = enrealloc(status, b->lines, b->capacity *
sizeof(*b->lines));
}
linelen = len;
- b->lines[b->nlines - 1].data = memcpy(emalloc(linelen + 1),
line, linelen + 1);
+ b->lines[b->nlines - 1].data = memcpy(enmalloc(status, linelen
+ 1), line, linelen + 1);
b->lines[b->nlines - 1].len = linelen;
}
free(line);
- if (b->lines && b->nlines && linelen && b->lines[b->nlines -
1].data[linelen - 1] != '\n') {
- b->lines[b->nlines - 1].data = erealloc(b->lines[b->nlines -
1].data, linelen + 2);
+ b->nolf = b->lines && b->nlines && linelen && b->lines[b->nlines -
1].data[linelen - 1] != '\n';
+ if (b->nolf) {
+ b->lines[b->nlines - 1].data = enrealloc(status,
b->lines[b->nlines - 1].data, linelen + 2);
b->lines[b->nlines - 1].data[linelen] = '\n';
b->lines[b->nlines - 1].data[linelen + 1] = '\0';
b->lines[b->nlines - 1].len++;
}
}
+
+void
+getlines(FILE *fp, struct linebuf *b)
+{
+ ngetlines(1, fp, b);
+}
diff --git a/patch.1 b/patch.1
new file mode 100644
index 000..df2bf63
--- /dev/null
+++ b/patch.1
@@ -0,0 +1,250 @@
+.Dd 2016-03-20
+.Dt PATCH 1
+.Os sbase
+.Sh NAME
+.Nm patch
