Re: sasc re-write
On Mon(17)/Jan/05 - , Simon 'corecode' Schubert wrote:
> hey, just a few comments
>
> On 16.01.2005, at 19:09, Liam J Foy wrote:
>
> >@@ -27,137 +28,165 @@
> > * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> > * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
> >USE OF
> > * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> >- *
> >+ *
>
> trailing whitespace addition
Fixed
>
> > * $FreeBSD: src/usr.bin/sasc/sasc.c,v 1.7.2.1 2000/06/30 09:47:52 ps
> >Exp $
> > * $DragonFly: src/usr.bin/sasc/sasc.c,v 1.4 2005/01/05 00:34:36
> >cpressey Exp $
> >+ *
>
> same here
Fixed
>
> > */
> >
> >+#include <sys/file.h>
> >+#include <sys/ioctl.h>
> >+#include <sys/types.h>
> >+
> > #include <err.h>
> >-#include <stdlib.h>
> >+#include <errno.h>
> > #include <stdio.h>
> >+#include <stdlib.h>
> > #include <unistd.h>
> >-#include <sys/file.h>
> >-#include <sys/ioctl.h>
> >+
> > #include <machine/asc_ioctl.h>
>
> i'd move this up after sys/*
Done
>
> >-int
> >-main(int argc, char **argv)
> >+/* Check given numerical arguments */
> >+static int
> >+getnum(const char *str)
> >+{
> >+ long val;
> >+ char *ep;
> >+
> >+ val = strtol(str, &ep, 10);
> >+ if (errno)
> >+ err(1, "strtol failed: %s", str);
>
> you don't reset errno to 0, so this might be not working. reading the
> man page it might be needed :/
Agreed. I believe we had a discussion ;) - fixed
>
> >+ if (str == ep || *ep != '\0')
> >+ errx(1, "invalid value: %s", str);
> >+
> >+ return((int)val);
> >+}
> >+
> >+static void
> >+asc_set(int fd, u_long asc_setting, int asc_value)
> > {
> >- char c;
> >- int fd;
> >+ if (ioctl(fd, asc_setting, &asc_value) < 0)
> >+ err(1, "ioctl failed setting %d", asc_value);
> >+
> >+ printf("Successfully set\n");
>
> you don't tell *what* got set :) this would be nice, but not directly
> neccessary.
>
> how about this:
>
> #define asc_set(fd, key, val) _asc_set(fd, key, val, #key)
> static void
> _asc_set(int fd, u_long asc_key, int asc_value, const char *asc_keyname)
> { ...
>
> >+static int
> >+asc_get(int fd, u_long asc_setting)
> >+{
> >+ int asc_value;
> >+
> >+ if (ioctl(fd, asc_setting, &asc_value) < 0)
> >+ err(1, "ioctl failed", asc_value);
>
> same here
Well, I think the fact we show what value is trying to be
set is enough. The user would know what he/she was trying to
set via what value they put it.
>
> cheers
> simon
>
> --
> /"\
> \ /
> \ ASCII Ribbon Campaign
> / \ Against HTML Mail and News
>
--
- Liam J. Foy
<liamfoy@sepulcrum.org>
討論串 (同標題文章)
完整討論串 (本文為第 4 之 10 篇)：