Hello,
On Wed, Oct 26, 2011 at 3:40 PM, Brewer, Janzen H. < [email protected]> wrote: > I'm new to augeas and I'm trying my hand at writing file descriptions. > I'd really appreciate your feedback on this one. It's written for > /etc/audit/auditd.conf. I also wrote a test. > > That's a very good start, and both lens and test pass augparse successfully, congrats! I'll just add a few comments: ------------------------- > augeas/lenses/auditd.aug: > ------------------------- > (* Auditd module for Augeas > Author: Janzen Brewer <[email protected]> > Based on Free Ekanayaka's dnsmasq module > > Reference: man auditd.conf (8) > > "[auditd.conf] should contain one configuration keyword per line, an equal > sign, and then followed by appropriate configuration information." > > *) > > module Auditd = > > autoload xfm > > (************************************************************************ > * USEFUL PRIMITIVES > *************************************************************************) > > It could be nice to use NaturalDocs comments, as seen in e.g. keepalived.aug. We generate documentation for lenses from the comments. See e.g. http://augeas.net/docs/references/lenses/files/keepalived-aug.html > let eol = Util.eol > let spc = Util.del_ws_spc > Nowadays, I'd prefer to use Sep.space, which is equivalent, but clearer (in my opinion). let comment = Util.comment > let empty = Util.empty > > let sep_eq = del /=/ "=" > This is Sep.equal. > let sto_to_eol = store /([^ \t\n].*[^ \t\n]|[^ \t\n])/ > > You could use "store Rx.space_in" here. > (************************************************************************ > * ENTRIES > *************************************************************************) > > (*let entry_re = /[A-Za-z0-9._-]+/*) > let key_value (kw:string) = [ key kw . spc . sep_eq . spc . sto_to_eol . > eol ] > This can be written using the build module as: let eq = del /[ \t]*=[ \t]*/ "=" let key_value (kw:string) = Build.key_value_line kw eq sto_to_eol > let entry = key_value "log_file" > | key_value "log_format" > | key_value "log_group" > | key_value "priority_boost" > | key_value "flush" > | key_value "freq" > | key_value "num_logs" > | key_value "max_log_file" > | key_value "max_log_file_action" > | key_value "space_left" > | key_value "space_left_action" > | key_value "action_mail_acct" > | key_value "admin_space_left" > | key_value "admin_space_left_action" > | key_value "disk_full_action" > | key_value "disk_error_action" > | key_value "dispatcher" > | key_value "disp_qos" > | key_value "name" > | key_value "name_format" > | key_value "tcp_listen_port" > | key_value "tcp_listen_queue" > | key_value "use_libwrap" > | key_value "tcp_client_ports" > | key_value "tcp_client_max_idle" > | key_value "tcp_max_per_addr" > | key_value "enable_krb5" > | key_value "krb5_principal" > | key_value "krb5_key_file" > But since Build.key_value_line accepts regexps as keys, I would suggest to define the keys as a union of regexps and pass it to key_value_line in one go. This will make a more efficient regexps, and simplify your code: let entry_re = "log_file" | "log_format" | "log_group" | "priority_boost" | "flush" | "freq" | "num_logs" | "max_log_file" | "max_log_file_action" | "space_left" | "space_left_action" | "action_mail_acct" | "admin_space_left" | "admin_space_left_action" | "disk_full_action" | "disk_error_action" | "dispatcher" | "disp_qos" | "name" | "name_format" | "tcp_listen_port" | "tcp_listen_queue" | "use_libwrap" | "tcp_client_ports" | "tcp_client_max_idle" | "tcp_max_per_addr" | "enable_krb5" | "krb5_principal" | "krb5_key_file" let entry = Build.key_value_line entry_re eq sto_to_eol The result in terms of performance, using the current code: time augparse -I . tests/test_auditd.aug real 0m0.780s user 0m0.632s sys 0m0.088s and using the new code: time augparse -I . tests/test_auditd.aug real 0m0.139s user 0m0.076s sys 0m0.012s You can also get rid of sep_eq (or redefine it if you prefer that variable name, I just wanted to make it clear that I used another variable here), spc, and eol. Note also that entry_re could be simply Rx.word. You would allow incorrect keywords, but it would parse fine and it would be easier to maintain the lens in the future. Cheers, Raphaël
_______________________________________________ augeas-devel mailing list [email protected] https://www.redhat.com/mailman/listinfo/augeas-devel
