I suppose you don't indent the printf debug lines to make them easier to spot. That's a sensible idea (although not one I've seen before). However, I treat Code Review answers as a "just before checkin" thing, so I'd suggest removing these by this point.
You pre-declare your variables. Don't do this; write them as close to point-of-use as reasonable.
Your NONE op seems to be rather useless - you could just use OR instead. So remove it.
Your
const char zero = '0';
const char _A = 'A';
const char _a = 'a';
variables might seem like a good adherence to "no magic constants", but in reality they have effectively no useful purpose. If you wanted to update your code to support other character sets, you'd want to redesign the whole parser anyway. IMHO, it's simpler to inline these.
You're not required to return 0; in modern C, and it's somewhat typical not to.
Your integer parser should be generic in the base. I suggest extracting this into a function like int parse_digit(char character, int base, int place):
int parse_digit(char character, int base, int place) {
int value;
if (character >= 'a') {
value = character - 'a' + 10;
}
else if (character >= 'A') {
value = character - 'A' + 10;
}
else {
value = character - '0';
}
return value * pow(base, place);
}
while (offs < len - 1) {
optmp += parse_digit(*(nend - offs), base, offs);
offs++;
}
base can be calculated with a simple lookup on ntype:
int type_to_base(type ntype) {
switch (ntype) {
case B02: return 2;
case B10: return 10;
case B16: return 16;
}
}
Although this makes the type enum rightly look silly: it's just an integer after all!
int flag_to_base(char flag) {
switch (flag) {
case 'b': return 2;
case 'd': return 10;
case 'x': return 16;
}
}
Note that this is a somewhat silly way to do things, though. First of all, pow takes doubles! I'd rather avoid that if possible. Further, this is more work than needs to be done. Instead, one can do this:
1*base^4 + 0*base^3 + 1*base^2 + 0*base^1 + 1*base^0 // how you're currently doing it
= (((1 * base + 0) * base + 1) * base + 0) * base + 1 // simpler way to do it
This can be done as:
int parse_digit(char character) {
if (character >= 'a') {
return character - 'a' + 10;
}
else if (character >= 'A') {
return character - 'A' + 10;
}
else {
return character - '0';
}
}
while (offs < len - 1) {
optmp *= base;
optmp += parse_digit(*(nend - offs));
offs++;
}
Parsing integers should be in another function, and can be slightly simplified:
long parse_int(char *start, char *end) {
int base = flag_to_base(*end);
long ret = 0;
for (char *digit = start; digit < end; digit++) {
ret *= base;
ret += parse_digit(*digit);
}
return ret;
}
I would also put application of optmp into a function, and stop calling it optmp (in general, most variables are temporary - stating this is pointless):
long apply(operator op, long left, long right) {
switch (op) {
case AND: return left & right;
case OR: return left | right;
case XOR: return left ^ right;
}
}
I would then change (generally) long to uint64_t and int to uint32_t (or even uint8_t) - there's no reason to be using variable-length integers in this day and age for these types, and you don't need signed values. When you do want variable-length values, you'd probably rather stick with size_t. (If you find these unwieldy, typedef them to u64, u32 and u8. Just don't use int and long 'cause they're prettier.)
Compiling with warnings, I get
calc.c: In function ‘apply’:
calc.c:18:1: warning: control reaches end of non-void function [-Wreturn-type]
}
^
calc.c: In function ‘flag_to_base’:
calc.c:26:1: warning: control reaches end of non-void function [-Wreturn-type]
}
^
This is because the new method allows better case analysis - we've forgotten to check that such operations are actually valid!
#include <math.h>
#include <stdint.h>
#include <stdio.h>
#include <string.h>
typedef enum {
AND = '&',
OR = '|',
XOR = '^',
} operator;
int apply(operator op, uint64_t *left, uint64_t right) {
switch (op) {
case AND: *left &= right; break;
case OR: *left |= right; break;
case XOR: *left ^= right; break;
default: return -1;
}
return 0;
}
int flag_to_base(char flag, uint8_t *base) {
switch (flag) {
case 'b': *base = 2; break;
case 'd': *base = 10; break;
case 'x': *base = 16; break;
default: return -1;
}
return 0;
}
int parse_digit(char digit, uint8_t *value) {
if ('a' <= digit && digit <= 'z') {
*value = digit - 'a' + 10;
}
else if ('A' <= digit && digit <= 'Z') {
*value = digit - 'A' + 10;
}
else if ('0' <= digit && digit <= '9') {
*value = digit - '0';
}
else {
return -1;
}
return 0;
}
int parse_int(uint64_t *ret, char *start, char *end) {
uint8_t base;
if (flag_to_base(*end, &base)) {
return -1;
}
*ret = 0;
for (char *digit = start; digit < end; digit++) {
uint8_t value;
if (parse_digit(*digit, &value) || value >= base) {
return -1;
}
*ret *= base;
*ret += value;
}
return 0;
}
int main(int argc, char **argv) {
uint64_t result = 0;
operator op = OR;
for (int i = 1; i < argc; i++) {
size_t len = strlen(argv[i]);
char *end = argv[i] + len - 1;
if (len == 1) {
op = *end;
continue;
}
uint64_t operand;
if (parse_int(&operand, argv[i], end)) {
printf("Can't parse argument '%s'\n", argv[i]);
return -1;
}
if (apply(op, &result, operand)) {
printf("Can't parse argument '%c'\n", op);
return -1;
}
}
printf("%li\n", result);
}
Note that I would be wary of casting to enums where this might result in an "invalid" enum, but at least it is well-defined behaviour.
For the Python, your
_bin = lambda x: bin(x)[2:] + 'b'
_dec = lambda x: str(x) + 'd'
_hex = lambda x: hex(x)[2:] + 'x'
can be just
_bin = "{:b}b".format
_dec = "{:d}d".format
_hex = "{:x}x".format
Your embedded loops can be simplified to
from itertools import product
for args in product(ops, types, range(RANGE), range(RANGE)):
test(*args)
You don't use time - remove it. Rather than rename subprocess to subp, I would directly import Popen and PIPE.
Use new-style formatting, please! Also, use eval instead of exec - using neither would be best, although its use is mildly understandable in this case (even if still saddening).
Don't call quit - this is a function only for interpreter usage. I actually suggest just asserting in this case.
Your subprocess call only wants output, so drop stdin=PIPE. Further, you can just call check_output:
import subprocess
from itertools import product
def test(op, t, x, y):
call = ["./calc", t(x), op, t(y)]
print call
data = subprocess.check_output(call)
assert eval("{}{}{}".format(x, op, y)) == int(data)
ops = "&|^"
types = "{:b}b".format, "{:d}d".format, "{:x}x".format
count = 20
for args in product(ops, types, range(count), range(count)):
test(*args)
