Thanks for the tips, Raphaël. Janzen
On 10/26/2011 09:59 AM, Raphaël Pinson wrote:
Hello, On Wed, Oct 26, 2011 at 3:40 PM, Brewer, Janzen H. <[email protected] <mailto:[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] <mailto:[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
