[Buildroot] [PATCH 0/2] Makedevs: add recursive option

[Buildroot] [PATCH 0/2] Makedevs: add recursive option

[Angelo Compagnucci]

This patch series add the recursive option to makedevs.

With recursive option will be possible to apply owner/permission
recursively to folders.

Angelo Compagnucci (2):
  package/makedevs: add recursive option
  docs/manual: documentation for recursive makedevs syntax

 docs/manual/makedev-syntax.txt | 10 +++++++++-
 package/makedevs/makedevs.c    | 23 +++++++++++++++++++++++
 2 files changed, 32 insertions(+), 1 deletion(-)

-- 
1.9.1

[Buildroot] [PATCH 1/2] package/makedevs: add recursive option

[Angelo Compagnucci]

This patch adds the possibilty to change owner/permission
of a folder recursively.

Signed-off-by: Angelo Compagnucci <angelo.compagnucci@gmail.com>
---
 package/makedevs/makedevs.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/package/makedevs/makedevs.c b/package/makedevs/makedevs.c
index ab90b93..e575427 100644
--- a/package/makedevs/makedevs.c
+++ b/package/makedevs/makedevs.c
@@ -34,6 +34,8 @@
 #ifndef __APPLE__
 #include <sys/sysmacros.h>     /* major() and minor() */
 #endif
+#include <ftw.h>
+
 
 const char *bb_applet_name;
 
@@ -332,6 +334,7 @@ void bb_show_usage(void)
 	fprintf(stderr, "Where name is the file name,  type can be one of:\n");
 	fprintf(stderr, "      f       A regular file\n");
 	fprintf(stderr, "      d       Directory\n");
+	fprintf(stderr, "      r       Directory recursively\n");
 	fprintf(stderr, "      c       Character special device file\n");
 	fprintf(stderr, "      b       Block special device file\n");
 	fprintf(stderr, "      p       Fifo (named pipe)\n");
@@ -364,6 +367,20 @@ void bb_show_usage(void)
 	exit(1);
 }
 
+bb_recursive(const char *full_name, uid_t uid, gid_t gid, unsigned int mode){
+
+	int chmod_chown(const char *fpath, const struct stat *sb,
+		int tflag, struct FTW *ftwbuf) {
+		if (chown(fpath, uid, gid) == -1) return -1;
+		if ((mode != -1)) {
+			if (chmod(fpath, mode) < 0) return -1;
+		}
+		return 0;
+	}
+
+	return nftw(full_name, chmod_chown, 20, FTW_MOUNT | FTW_PHYS);
+}
+
 int main(int argc, char **argv)
 {
 	int opt;
@@ -474,6 +491,12 @@ int main(int argc, char **argv)
 				ret = EXIT_FAILURE;
 				goto loop;
 			}
+		} else if (type == 'r') {
+			if (bb_recursive(full_name, uid, gid, mode) < 0) {
+				bb_perror_msg("line %d: recursive failed for %s", linenum, full_name);
+				ret = EXIT_FAILURE;
+				goto loop;
+			}
 		} else
 		{
 			dev_t rdev;
-- 
1.9.1

[Buildroot] [PATCH 1/2] package/makedevs: add recursive option

[Angelo Compagnucci]

Dear Buildroot developers,
Il 13/mar/2015 11:50, "Angelo Compagnucci" <angelo.compagnucci@gmail.com>
ha scritto:
>
> This patch adds the possibilty to change owner/permission
> of a folder recursively.
>
> Signed-off-by: Angelo Compagnucci <angelo.compagnucci@gmail.com>
> ---

Anybody willing to review this patch? I think it's very important to have
the option to set permission on folders recursively!

I think this patch is really simple and could be reviewed/accepted with a
minimal effort.

>  package/makedevs/makedevs.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>
> diff --git a/package/makedevs/makedevs.c b/package/makedevs/makedevs.c
> index ab90b93..e575427 100644
> --- a/package/makedevs/makedevs.c
> +++ b/package/makedevs/makedevs.c
> @@ -34,6 +34,8 @@
>  #ifndef __APPLE__
>  #include <sys/sysmacros.h>     /* major() and minor() */
>  #endif
> +#include <ftw.h>
> +
>
>  const char *bb_applet_name;
>
> @@ -332,6 +334,7 @@ void bb_show_usage(void)
>         fprintf(stderr, "Where name is the file name,  type can be one
of:\n");
>         fprintf(stderr, "      f       A regular file\n");
>         fprintf(stderr, "      d       Directory\n");
> +       fprintf(stderr, "      r       Directory recursively\n");
>         fprintf(stderr, "      c       Character special device file\n");
>         fprintf(stderr, "      b       Block special device file\n");
>         fprintf(stderr, "      p       Fifo (named pipe)\n");
> @@ -364,6 +367,20 @@ void bb_show_usage(void)
>         exit(1);
>  }
>
> +bb_recursive(const char *full_name, uid_t uid, gid_t gid, unsigned int
mode){
> +
> +       int chmod_chown(const char *fpath, const struct stat *sb,
> +               int tflag, struct FTW *ftwbuf) {
> +               if (chown(fpath, uid, gid) == -1) return -1;
> +               if ((mode != -1)) {
> +                       if (chmod(fpath, mode) < 0) return -1;
> +               }
> +               return 0;
> +       }
> +
> +       return nftw(full_name, chmod_chown, 20, FTW_MOUNT | FTW_PHYS);
> +}
> +
>  int main(int argc, char **argv)
>  {
>         int opt;
> @@ -474,6 +491,12 @@ int main(int argc, char **argv)
>                                 ret = EXIT_FAILURE;
>                                 goto loop;
>                         }
> +               } else if (type == 'r') {
> +                       if (bb_recursive(full_name, uid, gid, mode) < 0) {
> +                               bb_perror_msg("line %d: recursive failed
for %s", linenum, full_name);
> +                               ret = EXIT_FAILURE;
> +                               goto loop;
> +                       }
>                 } else
>                 {
>                         dev_t rdev;
> --
> 1.9.1
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20150405/257df3f5/attachment.html>

[Buildroot] [PATCH 1/2] package/makedevs: add recursive option

[Peter Korsgaard]

>>>>> "Angelo" == Angelo Compagnucci <angelo.compagnucci@gmail.com> writes:

 > Dear Buildroot developers,
 > Il 13/mar/2015 11:50, "Angelo Compagnucci" <angelo.compagnucci@gmail.com>
 > ha scritto:
 >> 
 >> This patch adds the possibilty to change owner/permission
 >> of a folder recursively.
 >> 
 >> Signed-off-by: Angelo Compagnucci <angelo.compagnucci@gmail.com>
 >> ---

 > Anybody willing to review this patch? I think it's very important to have
 > the option to set permission on folders recursively!

 > I think this patch is really simple and could be reviewed/accepted with a
 > minimal effort.

Sorry, I've been away quite a bit lately, I'll try to find time to
review later today.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH 1/2] package/makedevs: add recursive option

[Yann E. MORIN]

Angelo, All,

On 2015-03-13 11:50 +0100, Angelo Compagnucci spake thusly:
> This patch adds the possibilty to change owner/permission
> of a folder recursively.
> 
> Signed-off-by: Angelo Compagnucci <angelo.compagnucci@gmail.com>

Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

[I really only did a review and no actual tests]

As a side comment: maybe that would be interesting to submit a similar
patch for busybox (where makedevs originated); note however that busybox
has changed quite a lot since we forked makedevs...

Regards,
Yann E. MORIN.

> ---
>  package/makedevs/makedevs.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/package/makedevs/makedevs.c b/package/makedevs/makedevs.c
> index ab90b93..e575427 100644
> --- a/package/makedevs/makedevs.c
> +++ b/package/makedevs/makedevs.c
> @@ -34,6 +34,8 @@
>  #ifndef __APPLE__
>  #include <sys/sysmacros.h>     /* major() and minor() */
>  #endif
> +#include <ftw.h>
> +
>  
>  const char *bb_applet_name;
>  
> @@ -332,6 +334,7 @@ void bb_show_usage(void)
>  	fprintf(stderr, "Where name is the file name,  type can be one of:\n");
>  	fprintf(stderr, "      f       A regular file\n");
>  	fprintf(stderr, "      d       Directory\n");
> +	fprintf(stderr, "      r       Directory recursively\n");
>  	fprintf(stderr, "      c       Character special device file\n");
>  	fprintf(stderr, "      b       Block special device file\n");
>  	fprintf(stderr, "      p       Fifo (named pipe)\n");
> @@ -364,6 +367,20 @@ void bb_show_usage(void)
>  	exit(1);
>  }
>  
> +bb_recursive(const char *full_name, uid_t uid, gid_t gid, unsigned int mode){
> +
> +	int chmod_chown(const char *fpath, const struct stat *sb,
> +		int tflag, struct FTW *ftwbuf) {
> +		if (chown(fpath, uid, gid) == -1) return -1;
> +		if ((mode != -1)) {
> +			if (chmod(fpath, mode) < 0) return -1;
> +		}
> +		return 0;
> +	}
> +
> +	return nftw(full_name, chmod_chown, 20, FTW_MOUNT | FTW_PHYS);
> +}
> +
>  int main(int argc, char **argv)
>  {
>  	int opt;
> @@ -474,6 +491,12 @@ int main(int argc, char **argv)
>  				ret = EXIT_FAILURE;
>  				goto loop;
>  			}
> +		} else if (type == 'r') {
> +			if (bb_recursive(full_name, uid, gid, mode) < 0) {
> +				bb_perror_msg("line %d: recursive failed for %s", linenum, full_name);
> +				ret = EXIT_FAILURE;
> +				goto loop;
> +			}
>  		} else
>  		{
>  			dev_t rdev;
> -- 
> 1.9.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 1/2] package/makedevs: add recursive option

[Angelo Compagnucci]

Dear Yann E. MORIN,

2015-04-09 23:23 GMT+02:00 Yann E. MORIN <yann.morin.1998@free.fr>:
> Angelo, All,
>
> On 2015-03-13 11:50 +0100, Angelo Compagnucci spake thusly:
>> This patch adds the possibilty to change owner/permission
>> of a folder recursively.
>>
>> Signed-off-by: Angelo Compagnucci <angelo.compagnucci@gmail.com>
>
> Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
>
> [I really only did a review and no actual tests]
>
> As a side comment: maybe that would be interesting to submit a similar
> patch for busybox (where makedevs originated); note however that busybox
> has changed quite a lot since we forked makedevs...

I've not submitted upstream cause the code for makedevs is in
buildroot source tree ...
If you think it's important, I will submit not later the patch will accepted.

Thank you!

>
> Regards,
> Yann E. MORIN.
>
>> ---
>>  package/makedevs/makedevs.c | 23 +++++++++++++++++++++++
>>  1 file changed, 23 insertions(+)
>>
>> diff --git a/package/makedevs/makedevs.c b/package/makedevs/makedevs.c
>> index ab90b93..e575427 100644
>> --- a/package/makedevs/makedevs.c
>> +++ b/package/makedevs/makedevs.c
>> @@ -34,6 +34,8 @@
>>  #ifndef __APPLE__
>>  #include <sys/sysmacros.h>     /* major() and minor() */
>>  #endif
>> +#include <ftw.h>
>> +
>>
>>  const char *bb_applet_name;
>>
>> @@ -332,6 +334,7 @@ void bb_show_usage(void)
>>       fprintf(stderr, "Where name is the file name,  type can be one of:\n");
>>       fprintf(stderr, "      f       A regular file\n");
>>       fprintf(stderr, "      d       Directory\n");
>> +     fprintf(stderr, "      r       Directory recursively\n");
>>       fprintf(stderr, "      c       Character special device file\n");
>>       fprintf(stderr, "      b       Block special device file\n");
>>       fprintf(stderr, "      p       Fifo (named pipe)\n");
>> @@ -364,6 +367,20 @@ void bb_show_usage(void)
>>       exit(1);
>>  }
>>
>> +bb_recursive(const char *full_name, uid_t uid, gid_t gid, unsigned int mode){
>> +
>> +     int chmod_chown(const char *fpath, const struct stat *sb,
>> +             int tflag, struct FTW *ftwbuf) {
>> +             if (chown(fpath, uid, gid) == -1) return -1;
>> +             if ((mode != -1)) {
>> +                     if (chmod(fpath, mode) < 0) return -1;
>> +             }
>> +             return 0;
>> +     }
>> +
>> +     return nftw(full_name, chmod_chown, 20, FTW_MOUNT | FTW_PHYS);
>> +}
>> +
>>  int main(int argc, char **argv)
>>  {
>>       int opt;
>> @@ -474,6 +491,12 @@ int main(int argc, char **argv)
>>                               ret = EXIT_FAILURE;
>>                               goto loop;
>>                       }
>> +             } else if (type == 'r') {
>> +                     if (bb_recursive(full_name, uid, gid, mode) < 0) {
>> +                             bb_perror_msg("line %d: recursive failed for %s", linenum, full_name);
>> +                             ret = EXIT_FAILURE;
>> +                             goto loop;
>> +                     }
>>               } else
>>               {
>>                       dev_t rdev;
>> --
>> 1.9.1
>>
>> _______________________________________________
>> buildroot mailing list
>> buildroot at busybox.net
>> http://lists.busybox.net/mailman/listinfo/buildroot
>
> --
> .-----------------.--------------------.------------------.--------------------.
> |  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
> | +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
> | +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
> | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
> '------------------------------^-------^------------------^--------------------'



-- 
Profile: http://it.linkedin.com/in/compagnucciangelo

[Buildroot] [PATCH 1/2] package/makedevs: add recursive option

[Yann E. MORIN]

Angelo, All,

On 2015-04-10 07:39 +0200, Angelo Compagnucci spake thusly:
> 2015-04-09 23:23 GMT+02:00 Yann E. MORIN <yann.morin.1998@free.fr>:
> > On 2015-03-13 11:50 +0100, Angelo Compagnucci spake thusly:
> >> This patch adds the possibilty to change owner/permission
> >> of a folder recursively.
> >>
> >> Signed-off-by: Angelo Compagnucci <angelo.compagnucci@gmail.com>
> >
> > Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> >
> > [I really only did a review and no actual tests]
> >
> > As a side comment: maybe that would be interesting to submit a similar
> > patch for busybox (where makedevs originated); note however that busybox
> > has changed quite a lot since we forked makedevs...
> 
> I've not submitted upstream cause the code for makedevs is in
> buildroot source tree ...
> If you think it's important, I will submit not later the patch will accepted.

Well, I won't say it is "important". It all really depends from the
perspective.

For one, I believe this is an interesting feature, from which busybox
could well benefit. So it makes sense to submit it to busybox. Of
course, this is not really correlated to our own makedevs, except by
ancestry.

So, if you have spare CPU cycles, please go ahead. If not, then no
problem. That was just a suggestion! ;-)

Thanks!

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 1/2] package/makedevs: add recursive option

[Peter Korsgaard]

>>>>> "Angelo" == Angelo Compagnucci <angelo.compagnucci@gmail.com> writes:

 > This patch adds the possibilty to change owner/permission
 > of a folder recursively.

Thanks, a few comments:

 > +bb_recursive(const char *full_name, uid_t uid, gid_t gid, unsigned int mode){
 > +
 > +	int chmod_chown(const char *fpath, const struct stat *sb,
 > +		int tflag, struct FTW *ftwbuf) {
 > +		if (chown(fpath, uid, gid) == -1) return -1;
 > +		if ((mode != -1)) {
 > +			if (chmod(fpath, mode) < 0) return -1;
 > +		}
 > +		return 0;
 > +	}

I know you are doing this because nftw doesn't allow any extra arguments
to be passed to the function, but using GCC specific nested functions
isn't really nice.

Does E.G. clang support these? Alternatively we could port
recursive_action() from busybox.

It would also be good to stick some bb_error_msg in there so people can
get a hint about what fails.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH 1/2] package/makedevs: add recursive option

[Angelo Compagnucci]

Dear Peter,

2015-04-10 23:41 GMT+02:00 Peter Korsgaard <peter@korsgaard.com>:
>>>>>> "Angelo" == Angelo Compagnucci <angelo.compagnucci@gmail.com> writes:
>
>  > This patch adds the possibilty to change owner/permission
>  > of a folder recursively.
>
> Thanks, a few comments:
>
>  > +bb_recursive(const char *full_name, uid_t uid, gid_t gid, unsigned int mode){
>  > +
>  > +    int chmod_chown(const char *fpath, const struct stat *sb,
>  > +            int tflag, struct FTW *ftwbuf) {
>  > +            if (chown(fpath, uid, gid) == -1) return -1;
>  > +            if ((mode != -1)) {
>  > +                    if (chmod(fpath, mode) < 0) return -1;
>  > +            }
>  > +            return 0;
>  > +    }
>
> I know you are doing this because nftw doesn't allow any extra arguments
> to be passed to the function, but using GCC specific nested functions
> isn't really nice.
> Does E.G. clang support these?

Doh! I thought nested function could be a more widespread feature! Of
course, you are right, clang doesn't support nested functions.

The naive solution could be to add a global object, could be acceptable?
There are only three variables and could be nested inside a nice structure.

> Alternatively we could port
> recursive_action() from busybox.

/ * Unfortunately, while nftw(3) could replace this and reduce
 * code size a bit, nftw() wasn't supported before GNU libc 2.1,
 * and so isn't sufficiently portable to take over since glibc2.1
 * is so stinking huge.
 */

The only reason why they stick with an hand made recursive function
instead of nftw is to support older glibc 2.1 (!) that doesn't have
that function.
I think it's not our problem and backporting that old code is not a good idea!

What do you think?

> It would also be good to stick some bb_error_msg in there so people can
> get a hint about what fails.

Ok!

>
> --
> Bye, Peter Korsgaard



-- 
Profile: http://it.linkedin.com/in/compagnucciangelo

[Buildroot] [PATCH 1/2] package/makedevs: add recursive option

[Peter Korsgaard]

>>>>> "Angelo" == Angelo Compagnucci <angelo.compagnucci@gmail.com> writes:

hi,

>> I know you are doing this because nftw doesn't allow any extra arguments
 >> to be passed to the function, but using GCC specific nested functions
 >> isn't really nice.
 >> Does E.G. clang support these?

 > Doh! I thought nested function could be a more widespread feature! Of
 > course, you are right, clang doesn't support nested functions.

 > The naive solution could be to add a global object, could be acceptable?
 > There are only three variables and could be nested inside a nice structure.

Yes, or simply 3 globals (recursive_{uid,gid,mode}) to keep it simple.

 >> Alternatively we could port
 >> recursive_action() from busybox.

 > / * Unfortunately, while nftw(3) could replace this and reduce
 >  * code size a bit, nftw() wasn't supported before GNU libc 2.1,
 >  * and so isn't sufficiently portable to take over since glibc2.1
 >  * is so stinking huge.
 >  */

 > The only reason why they stick with an hand made recursive function
 > instead of nftw is to support older glibc 2.1 (!) that doesn't have
 > that function.
 > I think it's not our problem and backporting that old code is not a good idea!

Yes, that AND the fact that recursive_action takes a userData structure
that gets forwarded to the callbacks.

But yeah, just using nftw with 3 globals is simpler.

-- 
Venlig hilsen,
Peter Korsgaard 

[Buildroot] [PATCH 2/2] docs/manual: documentation for recursive makedevs syntax

[Angelo Compagnucci]

This patch adds the necessary documentation for the new makedevs
recursive syntax.

Signed-off-by: Angelo Compagnucci <angelo.compagnucci@gmail.com>
---
 docs/manual/makedev-syntax.txt | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/docs/manual/makedev-syntax.txt b/docs/manual/makedev-syntax.txt
index e02b79d..afb4ae2 100644
--- a/docs/manual/makedev-syntax.txt
+++ b/docs/manual/makedev-syntax.txt
@@ -24,6 +24,7 @@ There are a few non-trivial blocks:
 - +type+ is the type of the file, being one of:
   * f: a regular file
   * d: a directory
+  * r: a directory recursively
   * c: a character device file
   * b: a block device file
   * p: a named pipe
@@ -38,7 +39,14 @@ Let's say you want to change the permissions of a given file; using
 this syntax, you will need to put:
 
 ----
-/usr/bin/foobar f 644 0 0 - - -	- -
+/usr/bin/foobar f 644 0 0 - - - - -
+----
+
+Alternatively, if you want to change owner/permission of a folder
+recursively, you can put:
+
+----
+/usr/share/myapp r 750 myuser myuser - - - - -
 ----
 
 On the other hand, if you want to create the device file +/dev/hda+
-- 
1.9.1

[Buildroot] [PATCH 0/2] Makedevs: add recursive option

[Angelo Compagnucci]

Dear Buildroot Developers,

2015-03-13 11:50 GMT+01:00 Angelo Compagnucci <angelo.compagnucci@gmail.com>:
> This patch series add the recursive option to makedevs.
>
> With recursive option will be possible to apply owner/permission
> recursively to folders.
>
> Angelo Compagnucci (2):
>   package/makedevs: add recursive option
>   docs/manual: documentation for recursive makedevs syntax

Please give this patch series some love! I think that it iss very
important to have an option to set permission recursively. Think of
all that people that wants to serve html or any server side language
content in a secure manner, they can't actually!

Sincerely, Angelo.


-- 
Profile: http://it.linkedin.com/in/compagnucciangelo