From: Vladimir Davydov <vdavydov.dev@gmail.com> To: Olga Arkhangelskaia <arkholga@tarantool.org> Cc: tarantool-patches@freelists.org Subject: Re: [tarantool-patches] [PATCH v6] Configurable syslog destination Date: Wed, 1 Aug 2018 18:36:48 +0300 [thread overview] Message-ID: <20180801153648.wn26gmh66jsvwgx5@esperanza> (raw) In-Reply-To: <20180801150050.20937-1-arkholga@tarantool.org> On Wed, Aug 01, 2018 at 06:00:50PM +0300, Olga Arkhangelskaia wrote: > Added server option to syslog configuration. > Server option is responsible for log destination. At the momemt > there is two ways of usage:server=unix:/path/to/socket or > server=ipv4:port. If port is not set default udp port 514 is used. > If logging to syslog is set, however there is no sever options - A typo: s/sever/server > default location is used: Linux /dev/log and Mac /var/run/syslog. > > To monitor that the option is valid there is two tests in box-tap. This sentence is pointless. It is incumbent on the developer to write a functional test for each implemented feature. No point to mention it in the commit message. We only mention tests in the commit message if there's something peculiar about them. Please remove this sentence from the commit message. > +/** > + * Connect to remote syslogd using server:port. > + * @param server_address string of remote host. Please replace 'string' with 'address' in the comment ("string of remote host" doesn't sound right). > + * @retval not 0 Socket descriptor. > + * @retval -1 Socket error. > + */ > +static int > +syslog_connect_remote(const char *server_address) > +{ > + struct addrinfo *inf, hints, *ptr; > + const char *remote; > + char buf[10]; > + char *portnum, *copy; > + int fd = -1; > + int ret; > + > + copy = strdup(server_address); > + if (copy == NULL) { > + diag_set(OutOfMemory, strlen(server_address), "malloc", > + "stslog server address"); > + return fd; > + } > + portnum = copy; > + remote = strsep(&portnum, ":"); > + if (portnum == NULL) { > + snprintf(buf, sizeof(buf), "%d", SAY_SYSLOG_DEFAULT_PORT); > + portnum = buf; > + } > + memset(&hints, 0, sizeof(hints)); > + hints.ai_family = AF_INET; > + hints.ai_socktype = SOCK_DGRAM; > + hints.ai_protocol = IPPROTO_UDP; 'hints' are initialized, but never used (I assume they are supposed to be passed to getaddrinfo below). > + > + ret = getaddrinfo(remote, (const char*)portnum, NULL, &inf); A space is missing between 'const char' and the asterisk (*). Anyway, there's no need to convert 'portnum' to 'const char *', neither do you need to define 'remote' as 'const char *'. Please remove those const classifiers. > + if (ret < 0) { > + errno = EIO; > + diag_set(SystemError, "getaddrinfo:%s", A space is missing between the colon (:) and '%s'. Also, the issue is assigned to 1.10.2 (see the ticket) and hence is supposed to be pushed to branch 1.10 while your branch is based on 2.0. Please rebase on top of 1.10 and push - I want to make sure that tests pass on Travis CI before pushing it to the trunk.
prev parent reply other threads:[~2018-08-01 15:36 UTC|newest] Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-08-01 15:00 Olga Arkhangelskaia 2018-08-01 15:36 ` Vladimir Davydov [this message]
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20180801153648.wn26gmh66jsvwgx5@esperanza \ --to=vdavydov.dev@gmail.com \ --cc=arkholga@tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='Re: [tarantool-patches] [PATCH v6] Configurable syslog destination' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox