mg backup directory

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

mg backup directory

Lucas Gabriel Vuotto
Hi tech@,

mg(1)'s backup-to-home-directory writes backup files to `~/.mg.d'
according to the manpage. In order to expand the tilde, it uses a
custom function (expandtilde, fileio.c:700) which uses the pw entry for
the user name returned by getlogin(2). This can lead to an undesired
result if mg is run under another user via su.

For reading the startup file `~/.mg', it just tries to get HOME from
environment and falls back to a default location defined at compile
time if it can't get HOME. I think that it should do the same to
setup the backup directory, with no fall back. Inlined is a patch
to do that. While there, remove trailing spaces in fileio.c. A fall
back could be added if needed. In that case I suggest using `/tmp'
instead of a path defined at compile time.

If it isn't viable to merge the patch, I think the current behaviour
should be reflected in the manpage. I can also write a patch for that.

OK?

Cheers.

Index: fileio.c
===================================================================
RCS file: /cvs/src/usr.bin/mg/fileio.c,v
retrieving revision 1.103
diff -u -p -u -p -r1.103 fileio.c
--- fileio.c 28 Jul 2016 21:40:25 -0000 1.103
+++ fileio.c 13 May 2017 04:15:15 -0000
@@ -641,14 +641,15 @@ bkuplocation(const char *fn)
  int
  backuptohomedir(int f, int n)
  {
- const char *c = _PATH_MG_DIR;
- char *p;
+ char *home;

  if (bkupdir == NULL) {
- p = adjustname(c, TRUE);
- bkupdir = strndup(p, NFILEN);
- if (bkupdir == NULL)
- return(FALSE);
+ if ((home = getenv("HOME")) == NULL || *home == '\0')
+ return (FALSE);
+ if (asprintf(&bkupdir, _PATH_MG_DIR, home) == -1) {
+ bkupdir = NULL;
+ return (FALSE);
+ }

  if (mkdir(bkupdir, 0700) == -1 && errno != EEXIST) {
  free(bkupdir);
@@ -736,7 +737,7 @@ expandtilde(const char *fn)
  plen = strlcpy(path, pw->pw_dir, sizeof(path));
  if (plen == 0 || path[plen - 1] != '/') {
  if (strlcat(path, "/", sizeof(path)) >= sizeof(path)) {
- dobeep();
+ dobeep();
  ewprintf("Path too long");
  return (NULL);
  }
Index: pathnames.h
===================================================================
RCS file: /cvs/src/usr.bin/mg/pathnames.h,v
retrieving revision 1.1
diff -u -p -u -p -r1.1 pathnames.h
--- pathnames.h 18 Jun 2012 07:14:55 -0000 1.1
+++ pathnames.h 13 May 2017 04:15:15 -0000
@@ -6,6 +6,6 @@
   * standard path names
   */

-#define _PATH_MG_DIR "~/.mg.d"
+#define _PATH_MG_DIR "%s/.mg.d"
  #define _PATH_MG_STARTUP "%s/.mg"
  #define _PATH_MG_TERM "%s/.mg-%s"

Reply | Threaded
Open this post in threaded view
|

Re: mg backup directory

Lucas Gabriel Vuotto
Sorry, space got mangled in previous email.

Index: fileio.c
===================================================================
RCS file: /cvs/src/usr.bin/mg/fileio.c,v
retrieving revision 1.103
diff -u -p -u -p -r1.103 fileio.c
--- fileio.c 28 Jul 2016 21:40:25 -0000 1.103
+++ fileio.c 13 May 2017 04:15:15 -0000
@@ -641,14 +641,15 @@ bkuplocation(const char *fn)
 int
 backuptohomedir(int f, int n)
 {
- const char *c = _PATH_MG_DIR;
- char *p;
+ char *home;

  if (bkupdir == NULL) {
- p = adjustname(c, TRUE);
- bkupdir = strndup(p, NFILEN);
- if (bkupdir == NULL)
- return(FALSE);
+ if ((home = getenv("HOME")) == NULL || *home == '\0')
+ return (FALSE);
+ if (asprintf(&bkupdir, _PATH_MG_DIR, home) == -1) {
+ bkupdir = NULL;
+ return (FALSE);
+ }

  if (mkdir(bkupdir, 0700) == -1 && errno != EEXIST) {
  free(bkupdir);
@@ -736,7 +737,7 @@ expandtilde(const char *fn)
  plen = strlcpy(path, pw->pw_dir, sizeof(path));
  if (plen == 0 || path[plen - 1] != '/') {
  if (strlcat(path, "/", sizeof(path)) >= sizeof(path)) {
- dobeep();
+ dobeep();
  ewprintf("Path too long");
  return (NULL);
  }
Index: pathnames.h
===================================================================
RCS file: /cvs/src/usr.bin/mg/pathnames.h,v
retrieving revision 1.1
diff -u -p -u -p -r1.1 pathnames.h
--- pathnames.h 18 Jun 2012 07:14:55 -0000 1.1
+++ pathnames.h 13 May 2017 04:15:15 -0000
@@ -6,6 +6,6 @@
  * standard path names
  */

-#define _PATH_MG_DIR "~/.mg.d"
+#define _PATH_MG_DIR "%s/.mg.d"
 #define _PATH_MG_STARTUP "%s/.mg"
 #define _PATH_MG_TERM "%s/.mg-%s"

On 13/05/17 01:25, Lucas Gabriel Vuotto wrote:

> Hi tech@,
>
> mg(1)'s backup-to-home-directory writes backup files to `~/.mg.d'
> according to the manpage. In order to expand the tilde, it uses a
> custom function (expandtilde, fileio.c:700) which uses the pw entry for
> the user name returned by getlogin(2). This can lead to an undesired
> result if mg is run under another user via su.
>
> For reading the startup file `~/.mg', it just tries to get HOME from
> environment and falls back to a default location defined at compile
> time if it can't get HOME. I think that it should do the same to
> setup the backup directory, with no fall back. Inlined is a patch
> to do that. While there, remove trailing spaces in fileio.c. A fall
> back could be added if needed. In that case I suggest using `/tmp'
> instead of a path defined at compile time.
>
> If it isn't viable to merge the patch, I think the current behaviour
> should be reflected in the manpage. I can also write a patch for that.
>
> OK?
>
> Cheers.
>
> Index: fileio.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/mg/fileio.c,v
> retrieving revision 1.103
> diff -u -p -u -p -r1.103 fileio.c
> --- fileio.c    28 Jul 2016 21:40:25 -0000    1.103
> +++ fileio.c    13 May 2017 04:15:15 -0000
> @@ -641,14 +641,15 @@ bkuplocation(const char *fn)
>  int
>  backuptohomedir(int f, int n)
>  {
> -    const char    *c = _PATH_MG_DIR;
> -    char        *p;
> +    char    *home;
>
>      if (bkupdir == NULL) {
> -        p = adjustname(c, TRUE);
> -        bkupdir = strndup(p, NFILEN);
> -        if (bkupdir == NULL)
> -            return(FALSE);
> +        if ((home = getenv("HOME")) == NULL || *home == '\0')
> +            return (FALSE);
> +        if (asprintf(&bkupdir, _PATH_MG_DIR, home) == -1) {
> +            bkupdir = NULL;
> +            return (FALSE);
> +        }
>
>          if (mkdir(bkupdir, 0700) == -1 && errno != EEXIST) {
>              free(bkupdir);
> @@ -736,7 +737,7 @@ expandtilde(const char *fn)
>          plen = strlcpy(path, pw->pw_dir, sizeof(path));
>          if (plen == 0 || path[plen - 1] != '/') {
>              if (strlcat(path, "/", sizeof(path)) >= sizeof(path)) {
> -                dobeep();              
> +                dobeep();
>                  ewprintf("Path too long");
>                  return (NULL);
>              }
> Index: pathnames.h
> ===================================================================
> RCS file: /cvs/src/usr.bin/mg/pathnames.h,v
> retrieving revision 1.1
> diff -u -p -u -p -r1.1 pathnames.h
> --- pathnames.h    18 Jun 2012 07:14:55 -0000    1.1
> +++ pathnames.h    13 May 2017 04:15:15 -0000
> @@ -6,6 +6,6 @@
>   *    standard path names
>   */
>
> -#define    _PATH_MG_DIR        "~/.mg.d"
> +#define    _PATH_MG_DIR        "%s/.mg.d"
>  #define    _PATH_MG_STARTUP    "%s/.mg"
>  #define    _PATH_MG_TERM        "%s/.mg-%s"

Reply | Threaded
Open this post in threaded view
|

Re: mg backup directory

Lucas Gabriel Vuotto
Previous patch shall be ignored, as it was an ugly hack. Below is a patch that is simpler and fixes expandtilde instead, so it fixes the problem in other situations (writing files to ~, for example). The only thing that I'm not sure is whether to use getuid or geteuid. Any suggestion / explanation is welcome.

Index: usr.bin/mg/fileio.c
===================================================================
RCS file: /cvs/src/usr.bin/mg/fileio.c,v
retrieving revision 1.103
diff -u -p -u -p -r1.103 fileio.c
--- usr.bin/mg/fileio.c 28 Jul 2016 21:40:25 -0000 1.103
+++ usr.bin/mg/fileio.c 18 May 2017 17:53:03 -0000
@@ -723,15 +723,12 @@ expandtilde(const char *fn)
  return(ret);
  }
  if (ulen == 0) { /* ~/ or ~ */
- if ((un = getlogin()) != NULL)
- (void)strlcpy(user, un, sizeof(user));
- else
- user[0] = '\0';
+ pw = getpwuid(getuid());
  } else { /* ~user/ or ~user */
  memcpy(user, &fn[1], ulen);
  user[ulen] = '\0';
+ pw = getpwnam(user);
  }
- pw = getpwnam(user);
  if (pw != NULL) {
  plen = strlcpy(path, pw->pw_dir, sizeof(path));
  if (plen == 0 || path[plen - 1] != '/') {


On 13/05/17 01:32, Lucas Gabriel Vuotto wrote:

> Sorry, space got mangled in previous email.
>
> Index: fileio.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/mg/fileio.c,v
> retrieving revision 1.103
> diff -u -p -u -p -r1.103 fileio.c
> --- fileio.c 28 Jul 2016 21:40:25 -0000 1.103
> +++ fileio.c 13 May 2017 04:15:15 -0000
> @@ -641,14 +641,15 @@ bkuplocation(const char *fn)
>  int
>  backuptohomedir(int f, int n)
>  {
> - const char *c = _PATH_MG_DIR;
> - char *p;
> + char *home;
>
>   if (bkupdir == NULL) {
> - p = adjustname(c, TRUE);
> - bkupdir = strndup(p, NFILEN);
> - if (bkupdir == NULL)
> - return(FALSE);
> + if ((home = getenv("HOME")) == NULL || *home == '\0')
> + return (FALSE);
> + if (asprintf(&bkupdir, _PATH_MG_DIR, home) == -1) {
> + bkupdir = NULL;
> + return (FALSE);
> + }
>
>   if (mkdir(bkupdir, 0700) == -1 && errno != EEXIST) {
>   free(bkupdir);
> @@ -736,7 +737,7 @@ expandtilde(const char *fn)
>   plen = strlcpy(path, pw->pw_dir, sizeof(path));
>   if (plen == 0 || path[plen - 1] != '/') {
>   if (strlcat(path, "/", sizeof(path)) >= sizeof(path)) {
> - dobeep();
> + dobeep();
>   ewprintf("Path too long");
>   return (NULL);
>   }
> Index: pathnames.h
> ===================================================================
> RCS file: /cvs/src/usr.bin/mg/pathnames.h,v
> retrieving revision 1.1
> diff -u -p -u -p -r1.1 pathnames.h
> --- pathnames.h 18 Jun 2012 07:14:55 -0000 1.1
> +++ pathnames.h 13 May 2017 04:15:15 -0000
> @@ -6,6 +6,6 @@
>   * standard path names
>   */
>
> -#define _PATH_MG_DIR "~/.mg.d"
> +#define _PATH_MG_DIR "%s/.mg.d"
>  #define _PATH_MG_STARTUP "%s/.mg"
>  #define _PATH_MG_TERM "%s/.mg-%s"
>

Reply | Threaded
Open this post in threaded view
|

Re: mg backup directory

Florian Obser-2
On Fri, May 19, 2017 at 02:11:22PM -0300, Lucas Gabriel Vuotto wrote:
> Previous patch shall be ignored, as it was an ugly hack. Below is a patch that is simpler and fixes expandtilde instead, so it fixes the problem in other situations (writing files to ~, for example). The only thing that I'm not sure is whether to use getuid or geteuid. Any suggestion / explanation is welcome.

what is emacs doing in this case?

>
> Index: usr.bin/mg/fileio.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/mg/fileio.c,v
> retrieving revision 1.103
> diff -u -p -u -p -r1.103 fileio.c
> --- usr.bin/mg/fileio.c 28 Jul 2016 21:40:25 -0000 1.103
> +++ usr.bin/mg/fileio.c 18 May 2017 17:53:03 -0000
> @@ -723,15 +723,12 @@ expandtilde(const char *fn)
>   return(ret);
>   }
>   if (ulen == 0) { /* ~/ or ~ */
> - if ((un = getlogin()) != NULL)
> - (void)strlcpy(user, un, sizeof(user));
> - else
> - user[0] = '\0';
> + pw = getpwuid(getuid());
>   } else { /* ~user/ or ~user */
>   memcpy(user, &fn[1], ulen);
>   user[ulen] = '\0';
> + pw = getpwnam(user);
>   }
> - pw = getpwnam(user);
>   if (pw != NULL) {
>   plen = strlcpy(path, pw->pw_dir, sizeof(path));
>   if (plen == 0 || path[plen - 1] != '/') {
>
>
> On 13/05/17 01:32, Lucas Gabriel Vuotto wrote:
> >Sorry, space got mangled in previous email.
> >
> >Index: fileio.c
> >===================================================================
> >RCS file: /cvs/src/usr.bin/mg/fileio.c,v
> >retrieving revision 1.103
> >diff -u -p -u -p -r1.103 fileio.c
> >--- fileio.c 28 Jul 2016 21:40:25 -0000 1.103
> >+++ fileio.c 13 May 2017 04:15:15 -0000
> >@@ -641,14 +641,15 @@ bkuplocation(const char *fn)
> > int
> > backuptohomedir(int f, int n)
> > {
> >- const char *c = _PATH_MG_DIR;
> >- char *p;
> >+ char *home;
> >
> > if (bkupdir == NULL) {
> >- p = adjustname(c, TRUE);
> >- bkupdir = strndup(p, NFILEN);
> >- if (bkupdir == NULL)
> >- return(FALSE);
> >+ if ((home = getenv("HOME")) == NULL || *home == '\0')
> >+ return (FALSE);
> >+ if (asprintf(&bkupdir, _PATH_MG_DIR, home) == -1) {
> >+ bkupdir = NULL;
> >+ return (FALSE);
> >+ }
> >
> > if (mkdir(bkupdir, 0700) == -1 && errno != EEXIST) {
> > free(bkupdir);
> >@@ -736,7 +737,7 @@ expandtilde(const char *fn)
> > plen = strlcpy(path, pw->pw_dir, sizeof(path));
> > if (plen == 0 || path[plen - 1] != '/') {
> > if (strlcat(path, "/", sizeof(path)) >= sizeof(path)) {
> >- dobeep();
> >+ dobeep();
> > ewprintf("Path too long");
> > return (NULL);
> > }
> >Index: pathnames.h
> >===================================================================
> >RCS file: /cvs/src/usr.bin/mg/pathnames.h,v
> >retrieving revision 1.1
> >diff -u -p -u -p -r1.1 pathnames.h
> >--- pathnames.h 18 Jun 2012 07:14:55 -0000 1.1
> >+++ pathnames.h 13 May 2017 04:15:15 -0000
> >@@ -6,6 +6,6 @@
> >  * standard path names
> >  */
> >
> >-#define _PATH_MG_DIR "~/.mg.d"
> >+#define _PATH_MG_DIR "%s/.mg.d"
> > #define _PATH_MG_STARTUP "%s/.mg"
> > #define _PATH_MG_TERM "%s/.mg-%s"
> >
>

--
I'm not entirely sure you are real.

Reply | Threaded
Open this post in threaded view
|

Re: mg backup directory

Lucas Gabriel Vuotto
On 26/05/17 12:38, Florian Obser wrote:
> On Fri, May 19, 2017 at 02:11:22PM -0300, Lucas Gabriel Vuotto wrote:
>> Previous patch shall be ignored, as it was an ugly hack. Below is a patch that is simpler and fixes expandtilde instead, so it fixes the problem in other situations (writing files to ~, for example). The only thing that I'm not sure is whether to use getuid or geteuid. Any suggestion / explanation is welcome.
>
> what is emacs doing in this case?
>

It reads HOME from environment (from lisp--arghhh) if the path is ~/foo, or calls getpwnam if the path is ~user/foo.

Reply | Threaded
Open this post in threaded view
|

Re: mg backup directory

Lucas Gabriel Vuotto
bump because I forgot to CC florian@

On 26/05/17 18:11, Lucas Gabriel Vuotto wrote:
> On 26/05/17 12:38, Florian Obser wrote:
>> On Fri, May 19, 2017 at 02:11:22PM -0300, Lucas Gabriel Vuotto wrote:
>>> Previous patch shall be ignored, as it was an ugly hack. Below is a patch that is simpler and fixes expandtilde instead, so it fixes the problem in other situations (writing files to ~, for example). The only thing that I'm not sure is whether to use getuid or geteuid. Any suggestion / explanation is welcome.
>>
>> what is emacs doing in this case?
>>
>
> It reads HOME from environment (from lisp--arghhh) if the path is ~/foo, or calls getpwnam if the path is ~user/foo.

Reply | Threaded
Open this post in threaded view
|

Re: mg backup directory (bump)

Lucas Gabriel Vuotto
In reply to this post by Lucas Gabriel Vuotto
Bump.

Emacs gets HOME from environment here. I think that getting it from pw entry is more correct, but I can make a patch to behave like emacs if needed.

On 19/05/17 14:11, Lucas Gabriel Vuotto wrote:

> Previous patch shall be ignored, as it was an ugly hack. Below is a patch that is simpler and fixes expandtilde instead, so it fixes the problem in other situations (writing files to ~, for example). The only thing that I'm not sure is whether to use getuid or geteuid. Any suggestion / explanation is welcome.
>
> Index: usr.bin/mg/fileio.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/mg/fileio.c,v
> retrieving revision 1.103
> diff -u -p -u -p -r1.103 fileio.c
> --- usr.bin/mg/fileio.c    28 Jul 2016 21:40:25 -0000    1.103
> +++ usr.bin/mg/fileio.c    18 May 2017 17:53:03 -0000
> @@ -723,15 +723,12 @@ expandtilde(const char *fn)
>          return(ret);
>      }
>      if (ulen == 0) { /* ~/ or ~ */
> -        if ((un = getlogin()) != NULL)
> -            (void)strlcpy(user, un, sizeof(user));
> -        else
> -            user[0] = '\0';
> +        pw = getpwuid(getuid());
>      } else { /* ~user/ or ~user */
>          memcpy(user, &fn[1], ulen);
>          user[ulen] = '\0';
> +        pw = getpwnam(user);
>      }
> -    pw = getpwnam(user);
>      if (pw != NULL) {
>          plen = strlcpy(path, pw->pw_dir, sizeof(path));
>          if (plen == 0 || path[plen - 1] != '/') {
>