Client SOAP request

This is a SOAP request to fetch currency exchange rates by calling wsdl service.
The break of while loop is not elegant as it currently relies on knowing the last currency, but I could not find better solution by handling the error in case of no match. Recommendations and improvements are welcome! Thanks.

const std = @import("std");

const FXRate = struct {
    unit: []u8,
    currency: []u8,
    value: []u8,
};

var pos_end: usize = 0;

fn fetchRates(allocator: std.mem.Allocator) ![]u8 {
    const request_text: []const u8 =
        \\ <soapenv:Envelope xmlns:soapenv="http://schemas.xmlsoap.org/soap/envelope/" xmlns:web="http://www.mnb.hu/webservices/">
        \\      <soapenv:Header/>
        \\      <soapenv:Body>
        \\          <web:GetCurrentExchangeRates/>
        \\      </soapenv:Body>
        \\ </soapenv:Envelope>
    ;

    var client = std.http.Client{ .allocator = allocator };
    defer client.deinit();

    const uri = try std.Uri.parse("http://www.mnb.hu/arfolyamok.asmx?wsdl");
    var headers = std.http.Headers{ .allocator = allocator };
    defer headers.deinit();

    try headers.append("content-type", "application/xml");
    var request = try client.request(.POST, uri, headers, .{});
    defer request.deinit();

    request.transfer_encoding = .chunked;
    try request.start();
    try request.writer().writeAll(request_text);
    try request.finish();
    try request.wait();

    return try request.reader().readAllAlloc(allocator, 4096);
}

fn getTag(allocator: std.mem.Allocator, message: []u8, tag_name: []const u8) ![]u8 {
    var pos_start = std.mem.indexOfPos(u8, message, 0, tag_name) orelse unreachable;
    pos_start = pos_start + tag_name.len + 2; // 2 = length of ="
    pos_end = pos_start + 1;

    while (message[pos_end] != '\"') {
        pos_end += 1;
    }

    return allocator.dupe(u8, message[pos_start..pos_end]);
}

fn getTagValue(allocator: std.mem.Allocator, message: []u8) ![]u8 {
    var pos_start = std.mem.indexOfPos(u8, message, 0, "&gt;") orelse unreachable;
    pos_end = std.mem.indexOfPos(u8, message, 0, "&lt;") orelse unreachable;

    // 4 = length of &gt;
    return allocator.dupe(u8, message[pos_start + 4 .. pos_end]);
}

pub fn main() !void {
    var gpa = std.heap.GeneralPurposeAllocator(.{}){};
    const allocator = gpa.allocator();

    const message = try fetchRates(allocator);
    defer allocator.free(message);

    var fx_rates = std.ArrayList(FXRate).init(allocator);
    defer fx_rates.deinit();

    var tag_name: []const u8 = "Day date";
    std.debug.print("Date: {s}\n", .{try getTag(allocator, message, tag_name)});
    std.debug.print("Rate unit\tCurrency\tValue\n", .{});
    var pos_start = pos_end;

    while (true) {
        const unit = try getTag(allocator, message[pos_start..], "Rate unit");
        pos_start += pos_end;
        const currency = try getTag(allocator, message[pos_start..], "curr");
        pos_start += pos_end;
        const value = try getTagValue(allocator, message[pos_start..]);
        pos_start += pos_end;

        const r: FXRate = .{ .unit = unit, .currency = currency, .value = value };
        try fx_rates.append(r);

        if (std.mem.eql(u8, currency, "ZAR") == true) break;
    }

    for (fx_rates.items) |rate| {
        std.debug.print("{s}\t\t{s}\t\t{s}\n", .{ rate.unit, rate.currency, rate.value });
    }
}

Welcome!
While I don’t know anything about the SOAP request you are parsing, I can give you some general suggestions:

  1. You should deinit your allocator. If you don’t then you are creating leaks(here this is actually not a problem since your program exits). Additionally the GeneralPurposeAllocator performs leak checking on deinit, even showing stacktraces where you allocated the memory in debug.
var gpa = std.heap.GeneralPurposeAllocator(.{}){};
defer if(gpa.deinit() == .leak) {
    std.debug.print("Memory leak", .{});
};
  1. I personally find the global variable pos_end a bit confusing.
    I would suggest to create a struct that contains the message, pos_start and pos_end
    with member fuctions getTag() and getTagValue(),
    and fetchRates() as the init() function.
    This would allow you to simplify the main(), moving all the pos_start/pos_end related stuff into the respective function.
  2. Regarding the break in the while loop. I would just add a hasNextTag() function and check that in the beginning of the loop:
fn hasNextTag(message: []u8, tag_name: []const u8) bool {
   return std.mem.containsAtLeast(u8, message, 1, tag_name);
}
...
    while(hasNextTag(message[pos_start..], "Rate unit")) {...}
  1. if (std.mem.eql(u8, currency, "ZAR") == true) break;
    There is no need to compare bools. You can just plug it into the if directly:
    if (std.mem.eql(u8, currency, "ZAR")) break;
1 Like

Thanks IntegratedQuantum for the comments!

Memory leak actually was an issue because of allocator.dupe(). I guess I should just use ArenaAllocator() instead where deinit() frees all allocations at once?

The std.mem.containsAtLeast() is exactly what I was looking for thanks!

Yes I agree global variables are not preferred in general, I will work on your suggestion.

Yeah, you could do that.
However keep in mind that the ArenaAllocator tends to keep memory alive, even if you free it. For example with an ArenaAllocator your http headers, which you only used once in a function, will stick around in memory until you actually deinit() the arena.

So I would suggest only using the ArenaAllocator for the things you actually want to keep for longer, and using a GeneralPurposeAllocator for the short-lived stuff.
You can even have the ArenaAllocator use the GeneralPurposeAllocator in the background. That way you can catch if you leak the arena.

In my program I for example use one global GeneralPurposeAllocator and a bunch of local ArenaAllocators(for convenience) and FixedBufferAllocators(for performance).

1 Like

While an arena allocator could be a solution in this instance, I would recommend making sure you actually need an arena before using one. They are susceptible to other memory issues as well. In general, I recommend that you make sure you know why you’re using one allocation strategy over another.

For instance, if you have structures that themselves have allocations, you can easily overwrite those on accident with reusable memory caches (such as arenas) if you’re not careful. To be clear, this problem isn’t unique to arenas but it’s just to demonstrate that proper memory handling practices should be observed no matter what allocator you are using.

That said, FixedBufferAllocator (and stack based allocators in general) are definitely under appreciated.

1 Like